-
Notifications
You must be signed in to change notification settings - Fork 3.4k
cleanup series I - part I #2151
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
**all: folded to margins** README: used local image, expanded license into a small section CONTRIBUTING: added title relooper README: removed licensing data (root of repo contains license)
@@ -5,7 +5,7 @@ This is a helper tool which is designed to make it possible | |||
for other apps to read emscripten's configuration variables | |||
in a unified way. Usage: | |||
|
|||
em-config VAR_NAME | |||
em-config VAR_NAME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like these changes switched spaces to tabs? were there tabs in other places in those files? our convention is spaces actually.
I thought that tabs might actually be better, seeing as that tabs are clearer, and allow you to jump between indentations. In files where it could cause potential issues (bat files), I have left tabs as they are. |
As for the licenses, I felt that for new visitors (ie: more or less everyone who reads the Perhaps I could remove some of that portion from the |
This reverts commit b0bc4b4. altering `emcc` this strongly will break many other pulls and will cause a nightmare in terms of merging
Even though tabs are better (for reasons outlined above),
|
I see your point about more info being useful in the readme. But we should keep all info in LICENSE. So I guess having it in both places as you did is ok then. |
Looks good overall, except for the tabs which we should keep as spaces, to match our convention everywhere else. |
Fixed. |
@kripken I reverted the tabs to spaces, so I think this is ready for merging. |
@kripken Is there anything else you wanted me to change? |
Looks good, please just rebase so that there are no merge commits. |
This reverts commit d09166a.
@kripken rebased and force-pushed |
some basic cleanups as part of #20, mainly: