-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add ability to compare between fork point #46
Conversation
…commit on current branch" This reverts commit e455529
…he branch changes should be made against. Currently supports comparing against PreviousCommit and ForkCommit
…t the end of each line, executeAndParseFirst returns the first line of stdout without the new line character - lets us clean up some common code to remove this new line character etc
This reverts commit ef597ae
Revert "App Change" This reverts commit 4a34ec51
Codecov Report
@@ Coverage Diff @@
## main #46 +/- ##
============================================
+ Coverage 44.05% 46.15% +2.09%
- Complexity 28 39 +11
============================================
Files 8 11 +3
Lines 404 429 +25
Branches 88 95 +7
============================================
+ Hits 178 198 +20
- Misses 197 200 +3
- Partials 29 31 +2
Continue to review full report at Codecov.
|
Wow thank you for all the hard work! I'll review in the next few days. Doing holiday stuff now |
No need to thank, was a pleasure to contribute 😄 Sounds good! Have a merry Christmas! 🎅 |
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.
Again, thank you for this PR and especially for increasing code coverage!
I have 1 small request.
...r/src/main/kotlin/com/dropbox/affectedmoduledetector/commitshaproviders/CommitShaProvider.kt
Outdated
Show resolved
Hide resolved
...c/test/kotlin/com/dropbox/affectedmoduledetector/commitshaproviders/CommitShaProviderTest.kt
Show resolved
Hide resolved
I don't have write access to this repo. Is someone able to merge this for me? Or alternatively, grant me write access |
Addresses #4
Adds the ability to compare between the fork point of the current branch.
Added a compareFrom configuration so the user is able to enter the desired behaviour when comparing the current branch changes to either:
I made a
CommitShaProvider
interface which contains a function which takes theCommandRunner
and returns aSha
. I would have preferred to have theCommitShaProvider
return a single command without the CommandRunner parameter (as suggested here) but nested commands aren't supported by theProcessBuilder
(e.g.) so I opted for the current solution.