search

Financial-Times / nori

🍙 exploratory command-line tool to make changes across multiple repositories & track their progress
MIT License
11 stars 0 forks source link

CPP-261 applied git cherry before pushing branches #130

Closed emortong closed 4 years ago

emortong commented 4 years ago

Important: https://github.com/Financial-Times/tooling-helpers/pull/86 should be merged first

Ticket: CPP-261

A git cherry command has been added before pushing branches to their remotes to check if the branch has any new commits compared to its upstream branch, if there aren't any, the pushing of the branch is skipped and a message is logged.

This is to avoid pushing branches where commits have not been made - as a result of the transformation script not making any changes or it resulting in an error.

andygout commented 4 years ago

check if the branch has any new commits compared to its upstream branch, if there aren't any, the pushing of the branch is skipped and a message is logged

I imagine this scenario (pushing is skipped and message is logged) is also applied if there is not yet an upstream branch whatsoever?

apaleslimghost commented 4 years ago

if there is not yet an upstream branch whatsoever?

there'll always be an upstream branch, git push will create it if there isn't one

andygout commented 4 years ago

But if pushing is skipped then surely that means git push does not happen and the branch is not created?

Though I have a distinct feeling I have missed something.

apaleslimghost commented 4 years ago

i don't really know what the problem is you're describing? if there are no commits there's no push and then no upstream branch. if you come back later and there are new commits it will create that branch in the push. is there a problem with that scenario or are you describing something else?

andygout commented 4 years ago

Please ignore all my comments. Re-reading them I cannot recall whatsoever what I was trying to ask.

apaleslimghost commented 4 years ago

since the entire @financial-times/git module is mocked by Jest, you'll need to add specific dummy resolve values to the tests for push-branches, and it would be good to see a test for this case