Fix overriding notes bug and add tests #3
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* add rspec
* update README to mention testing
* simplify build, launcher, listener passing w/ BuildContext
* simplify logging and execing with Builder mixin
* add ConcurrentModificationError
* move constants to Constants
* switch to exception throwing instead of returning false
* fix two {:raise => true} issues
* change retry loop to begin-rescue-retry
* move note generation to own class
* couple renames and brought tests up to changes
…-git-notes-plugin into fix_overriding_notes_bug Conflicts: models/git_note.rb models/git_notes_publisher.rb spec/models/git_notes_publisher_spec.rb spec/spec_helper.rb
| @@ -0,0 +1,24 @@ | |||
| require 'singleton' | |||
There was a problem hiding this comment.
Why is this a singleton?
There was a problem hiding this comment.
And why separate it out from the Builder?
|
@tucker250: Addressed concerns in latest commit. Merging. |
bfulton
added a commit
that referenced
this pull request
Apr 11, 2012
Fix overriding notes bug and add tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@bfulton - First pass at a fix for losing git note history, and a refac to make it possible to unit test the plugin. The core change to fix the bug was to make the git note addition work like this:
git notes --ref=jenkins show HEADSince a race is possible here if two workers are adding a note to the same commit, if step 4 fails then we restart from step one. Note that the push in step 4 is not forced, and the pull in step 1 is. GitNotesPublisher will retry up to 3 times before giving up.
One open question is what "giving up on a git note" should entail. Should the build fail?