Improve Code Readability and Aesthetics

[kripken]

Help with making the code prettier would be great. It is really easy to improve this, just changing function names would be a good start. Bigger refactorings are also possible of course.

Emscripten has tons of automatic tests, so we can make these changes with no risk of regressions.

The reason I don't do more of this myself, is I am too used to the code to know what stands out as unreadable and in need of cleanup.

[crgosselin]

Hey Kripken, definitely willing to go through it all and start making it more aesthetically pleasing. As you said, I think it'll be a nice start to diving into the project. Cheers

[kripken]

Hey crgosselin, that would be great! Let me know if I can help explain stuff in the code that isn't clear.

Note that all new development is currently being done on the llvm-svn branch - which should merge soon to master. So best to work there, although likely there wouldn't be a problem anyhow.

[haneefmubarak]

If no one is working on this bug, may I take it as my first bug?

[kripken]

Feel free to help out here. I guess this bug is very old though and very general, it might help to focus on some specific area in the code.

[haneefmubarak]

Hey, I need some help with a few things before I start, so could we chat, perhaps on freenode or something?

[haneefmubarak]

Actually, here are the things I'd like:

• A branch on the main repo that I can push to (or something that automatically pulls from my branch) - cleanup sounds good
• A basic description of what is in the various directories - oneliners for each should do it
• A list of external dependencies for the testing suite (if any)

That ought to be all for now.

[kripken]

I am often on the emscripten irc channel, we can talk other. others that know about the code are there as well.

Not sure what a new branch would help, that pull requests do not do?

Main dirs:

• cmake - not sure, cmake people would know
• docs - documentation
• patches - old, can be removed or moved somewhere less prominent perhaps
• scons-tools - not sure, scons people would know
• src - main JS compiler
• system - system headers, libraries, etc.
• tests - test suite
• third_party - code under another license than ours that we use, like closure compiler
• tools - utility scripts, toolchain stuff

Test suite depends on the usual stuff as normal emscripten, but it does use all code paths. So it needs things like crunch. I can't think of anything else offhand.

[juj]

The cmake directory contains the compiler toolchain definition file for
Emscripten for CMake - also, the directory structure inside that directory
needs to be specific like it is due to hardcoded requirements in CMake
itself. CMake is used with the 'emconfigure cmake' command line to
configure build system from CMakeLists.txt files.

The Scons directory is similar, but for the Scons build system.

2014-02-24 22:42 GMT+02:00 Alon Zakai notifications@github.com:

I am often on the emscripten irc channel, we can talk other. others that
know about the code are there as well.

Not sure what a new branch would help, that pull requests do not do?

Main dirs:

• cmake - not sure, cmake people would know
• docs - documentation
• patches - old, can be removed or moved somewhere less prominent
  perhaps
• scons-tools - not sure, scons people would know
• src - main JS compiler
• system - system headers, libraries, etc.
• tests - test suite
• third_party - code under another license than ours that we use, like
  closure compiler
• tools - utility scripts, toolchain stuff

Test suite depends on the usual stuff as normal emscripten, but it does
use all code paths. So it needs things like crunch. I can't think of
anything else offhand.

Reply to this email directly or view it on GitHubhttps://github.com//issues/20#issuecomment-35933604
.

[haneefmubarak]

@juj Would it be possible for the cmake and scons-tools directories to be shifted into a build-tools directory?

[juj]

Is there a need to do that? I would not like to do so, as it could break existing toolchains people may have set up.

[haneefmubarak]

@kripken Having a branch would make it much easier to work with as master and incoming will likely diverge fairly often from cleanup and merging back into incoming will likely become less frequent as time goes on.

Also, as more significant changes start to occur, it may be useful to allow others to work on cleanup too.

[haneefmubarak]

@juj The top level directory is fairly crowded, to be honest. Although it may break up a fair number of toolchains, fixing said chains will be trivial as only the path would need to be edited.

[haneefmubarak]

@kripken Could you assign me the bug? thx.

[juj]

Merging the two directories will remove one item from the top of the tree. I don't think that wins much. We should not break anything for other people for just aesthetics reasons.

[haneefmubarak]

On the other hand, more than that will be moved and every file counts. Most of the top-level files will probably be moved into core.

Obviously, this will be a while, so I don't mind adding an FAQ file or a section on fixing anything that might go wrong.

[kripken]

I actually don't see a way to assign people to bugs anymore on github. Did they change or remove that? Anyhow, we weren't really using that field anyhow.

About a branch, I think we should not diverge from incoming for long anyhow. Small pull requests into incoming are the only practical thing, otherwise merging in older pulls of widespread changes will just be too hard.

I lean towards @juj's opinion on the toplevel dir issue.

[haneefmubarak]

Ok. Will not merge build directories.

[haneefmubarak]

While I'm at it, do you think adding comments to the code where there aren't any would be worth the time?

[kripken]

I guess it depends on the specifics. Usually I think comments are important only to avoid confusion that could otherwise happen, so if the code itself is very straightforward it should not be needed. But if you find places that look confusing, they might benefit from comments.

[haneefmubarak]

I'm extremely sorry to say that due to other commitments, I have not been able to contribute to the project along these lines over the past few weeks.

These commitments, in addition to school and my current relative lack of experience with python and js, mean that I will likely not be able to make any more contributions for the foreseeable future.

As such, I'd like to say that this bug is free for anyone else to pick up on from where I have left off.

It was nice working on this though, and it did give me a good amount of exp. outside of my comfort zone, and for that, I'd like to thank @kripken and @juj for the help, advice and corrections along the way.

• Haneef

[kripken]

Thanks for your contributions!

On Mon, Apr 14, 2014 at 11:22 PM, Haneef Mubarak
notifications@git.luolix.topwrote:

I'm extremely sorry to say that due to other commitments, I have not been
able to contribute to the project along these lines over the past few weeks.

These commitments, in addition to school and my current relative lack of
experience with python and js, mean that I will likely not be able to
make any more contributions for the foreseeable future.

As such, I'd like to say that this bug is free for anyone else to pick
up on from where I have left off.

It was nice working on this though, and it did give me a good amount of
exp. outside of my comfort zone, and for that, I'd like to thank @kripkenhttps://github.com/kripkenand
@juj https://github.com/juj for the help, advice and corrections along
the way.

• Haneef

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/20#issuecomment-40449484
.

[juj]

I'd prefer closing this, since this is not a clear bug that can be ever called as fixed?

[kripken]

Sure.