search

Teun / git-flow-vis

81 stars 12 forks source link

Code refactoring #51

Open remie opened 7 years ago

remie commented 7 years ago

The code base is becoming somewhat large and has a mixture of public and private API functions and variables. Maybe it is time for a bit of cleanup:

  1. Start the code base with the public API functions.
  2. Limit the size of public API functions for readability, and move larger chunks of code to private support functions.
  3. Create a 'utils' object which has all the private support functions.
  4. Re-evaluate the support methods...there are now multiple public / private API's with "draw" in their name, some which are part of the "drawing" object and some are directly on the main class.
  5. Have a more clear distinction between global variables and local variables. Perhaps replace some global variables with function parameters.
Teun commented 7 years ago

Should we drop backward compatibility? I think that will be hard. But I would like to limit it to renaming and moving stuff in the interface.

2017-04-10 13:55 GMT+02:00 Remie Bolte notifications@github.com:

The code base is becoming somewhat large and has a mixture of public and private API functions and variables. Maybe it is time for a bit of cleanup:

  1. Start the code base with the public API functions.
  2. Limit the size of public API functions for readability, and move larger chunks of code to private support functions.
  3. Create a 'utils' object which has all the private support functions.
  4. Re-evaluate the support methods...there are now multiple public / private API's with "draw" in their name, some which are part of the "drawing" object and some are directly on the main class.
  5. Have a more clear distinction between global variables and local variables. Perhaps replace some global variables with function parameters.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Teun/git-flow-vis/issues/51, or mute the thread https://github.com/notifications/unsubscribe-auth/AAeIT1ycy-DWiAUEGCV-x4GernYUaWtZks5ruhicgaJpZM4M4rfH .

remie commented 7 years ago

But I would like to limit it to renaming and moving stuff in the interface.

I'm not entirely sure what this means :) I also don't think this will drop backwards compatibility. The "public" API is already documented in the README and we can keep these methods with their functionality.

Teun commented 7 years ago

:) If you think we can keep that public API 100% backward compatible, it means nothing. I thought that maybe some of the naming in that API was also not very consistent or logical.

2017-04-10 15:11 GMT+02:00 Remie Bolte notifications@github.com:

But I would like to limit it to renaming and moving stuff in the interface.

I'm not entirely sure what this means :) I also don't think this will drop backwards compatibility. The "public" API is already documented in the README and we can keep these methods with their functionality.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Teun/git-flow-vis/issues/51#issuecomment-292944712, or mute the thread https://github.com/notifications/unsubscribe-auth/AAeIT6I3Px_zaueze5rgxy7FbjDzk73cks5ruip4gaJpZM4M4rfH .