allow promotion to release without code changes -- fixes #108 #111
Conversation
|
There were 480 failing tests before?! That's quite strange. Will have to take a look. |
vivin
left a comment
vivin
left a comment
There was a problem hiding this comment.
BTW, would you be able to modify this PR to merge into the develop branch instead of master?
| [false, true, false, null, true], | ||
| [false, true, false, null, false], | ||
| [false, true, true, null, true], | ||
| [false, true, true, null, false]]).findAll { |
There was a problem hiding this comment.
Since we're basically ignoring promoteToRelease here, you can update this test to remove that flag entirely.
It would be useful to add a test for the happy path -- basically making sure that you are able to promote to pre-release without making any commits.
Additionally, it would be useful to add a test to see how it handles having a pre-release and release tag on a commit. I think getLatestTagOnReference in VersionUtils takes care it by sorting and pulling out the latest version even if there are multiple tags on them. But since this plugin has never explicitly dealt with that case I think it would be useful to add a test.
Something that is potentially confusing with this change is that even if someone checks out a pre-release tag, it will use the release version. However, inspecting the tags should show them pointing to the same commit. Additionally, this could be easily detected and a message could be printed out to inform the user.
There was a problem hiding this comment.
I'm not completely comfortable altering this test signature, as it's confusing with all the bitwise-shifting, etc. Would you mind fixing it?
There was a problem hiding this comment.
I can look at it.
There's still the issue of failing tests that I need to figure out.
I need to re-learn the testing framework again -- another contributor wrote it and so I am not super familiar with it.
There was a problem hiding this comment.
Unfortunately I've been busy with my PhD and haven't had a chance to look at this at all. I'm not sure when I will be able to.
|
@JoshMcCullough what version of gradle are you using? The project was using a really old version and I'm noticing intermittent failures after I upgraded to Gradle 6 (had to update some config too). |
|
I'll take a look at this tomorrow. |
|
Sorry for sneaking in but are there any news or hope to have this pull merged? Would be a really nice feature to have. |
|
Oops, looks like I said I'd look into something then never did. But I agree, I'd love to have this merged in. |
|
I started looking at this this weekend but I can barely get the tests to run. A lot are failing (as mentioned, but my number was off originally). I'm not familiar with Spock (I use Mockito) so I'm not sure how to fix things w/out investing a lot of time. |
|
Ran into this again just now. :( |
3 participants
Proposed fix for #108. Needs review.
Note -- excluded 4 tests (which otherwise would now fail) from
VersionUtilsI think it is correct but should be verified as well! Also note that there were 480 failing tests before this change which are still failing.