search

rubenv / grunt-git

Git commands for grunt.
MIT License
227 stars 83 forks source link

Automate conversion of options to arguments #70

Closed dylancwood closed 10 years ago

dylancwood commented 10 years ago

As discussed in PR #61, this pull request creates a more structured system for converting task-options to CLI flags and arguments. After I tweaked my original solution to accommodate the various use cases, I decided that it would be easiest to put the bulk of the logic in a separate module that is reusable by other projects.

This PR uses flopmang, a module which I wrote just for this purpose. It is fully unit-tested, and I encourage you to look over the code when you have a chance: https://github.com/dylancwood/flopmang.

I tried to keep the changes here small in scope by not modifying any of the tests (except one, see notes in-line) or adding any new options. Hopefully this gives you high confidence that the changes do not affect the functionality of the code.

Once this is merged (when you approve, of course), it should be even easier for me and other collaborators to add more tasks and options to continue expanding this module's coverage of the git CLI.

Please let me know if you have any questions or concerns.

rubenv commented 10 years ago

Really good stuff. The fact that the test suite keeps passing (aside from two minor details, but they're OK) despite this big change is a testament to the quality of it.

I think you'll agree that this is a huge change, not something we can push as an update to 0.2.x.

Unless there's a good reason not to, I suggest we merge this to master and then work at ensuring #44 is fine, to lead up to a new big release.

dylancwood commented 10 years ago

That sounds good.

We can also make a non -production release, and ask users to test with their Grunt workflows in order to catch any bugs that might have been missed by the unit tests.

dylancwood commented 10 years ago

I'm glad that you like it.

I'm happy to help with #44, but adding all git options will be a tedious task. Would you be willing to split up the git commands? Thanks!

rubenv commented 10 years ago

I'm happy to help with #44, but adding all git options will be a tedious task. Would you be willing to split up the git commands?

We don't need all of them, I only really care about throwing out the ones where we differ and keeping the ones we already support. Any missing flags can be added later on as future improvements.

dylancwood commented 10 years ago

That sounds good. I will start working on #44 after this gets merged into master.

rubenv commented 10 years ago

Merged! Fantastic work.

Now that you've written most of the module (in its current state), would you be interested in co-maintaining it? I'll add you to the git repo, so you can commit to it directly for small fixes.

Do send PRs for bigger changes / things you're unsure of (or need a second pair of eyes on). And just ping me to roll releases.

Psyched, this is a big step forward for the module.

dylancwood commented 10 years ago

Thanks @rubenv! I'm psyched as well. I am happy to accept your offer of co-maintaining this library, and will still continue to use PRs for most changes: I'm a firm believer in code review and audit trails. I'll try to keep each PR small from now on so that it is quick to review.

rubenv commented 10 years ago

:+1: