-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: co-authors not cleared from message template (.gitmessage
) on switch from mob to solo
#89
Fix: co-authors not cleared from message template (.gitmessage
) on switch from mob to solo
#89
Conversation
.gitmessage
) on switch from mob to solo
this looks good and the change is along the lines I had in mind. there is something broken in the build scripts however; the feature tests failed and the build still passed. I am looking into that and will get back to you later today. |
demonstrates davidalpert#89
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.
@teezzan thank you for your contribution!
Your code change in 8b5c24f looks good.
I have a couple of notes about the testing and PR checks that I want us to address before merging your change:
- the test pattern in this repo includes both golang unit tests and also ruby/cucumber acceptance tests
- the unit tests pass with your code change but the acceptance tests fail since there are currently some acceptance tests around the
.gitmessage
file - the checks in this PR didn't fail due to PR build is passing with failing steps #90 which I investigated and fixed this morning
- the
.feature
file you added here appears to be testing that the core function of adding the Co-Authored-By tag still works after your change; this behavior is already covered by many other tests so you can remove it.- here is an example of the kind of test I was thinking to write before making any changes, demonstrating expected behavior and failing because of this bug: test: example bug feature #92
- note that this step and others like it in other feature files will need to be removed in order to pass with 8b5c24f
Requested changes:
- remove
features/git-mob/mob-init-persists-coauthor.feature
- please rebase on top of
bug-feature-example
(from test: example bug feature #92) to pick up the PR build fixes and that example feature file demonstrating the bug - run all the tests locally and fix broken specs by removing all the feature expectations that
~/.gitmessage
reflects the mob members
@teezzan do you think you will have time to get to the requested changes this month? if not, I can approve this PR as-is (since the code change resolves the bug) and then resolve any failed tests as housekeeping. |
On it, @davidalpert . Sorry for being AWOL. |
a2fe4c8
to
b272e53
Compare
This PR contains the following fixes:
~/.gitmessage
.@davidalpert Please check it out.