-
Notifications
You must be signed in to change notification settings - Fork 20
Conversation
We don't need to push the commit if it is available in the remote repo already. This is a relevant case when rolling back to a previous commit.
Only run 'git init' and config change if there is no .git directory yet
We don't need to assign those if we abort() later on
@Turbo87 Sorry I haven't had time to check this yet. I do appreciate the pull request. I'll test this in the next few days. Happy New Year! |
don't worry, I'm on skiing vacation at the moment. take your time ;) |
warn_only=True, quiet=True).succeeded: | ||
return True | ||
|
||
|
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.
This might not be necessary since git will do the right thing if the remote commit that is being pushed is already in the remote immutable object store.
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.
you're right about that, but my aim was to keep the console output clean. that's basically why I check if an action is necessary before performing it.
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.
👍 let's keep the change.
Overall I think think these changes are great. I do have a few nitpicks that I mentioned above. I think we need to add a test suite for this tool so we don't need to manually test changes. I opened #7 for this. |
A test suite would be nice. I'm not entirely sure how to test something like this, but I guess it won't be entirely impossible ;) Do you want to have the test suite before merging this or is that independent? |
Totally independent. |
Gracias :) |
see the commits for what exactly has been changed. the public API should be the same as before, but will skip a few unnecessary commands.