Conversation
Collaborator
shemuwel
commented
Mar 28, 2024
shemuwel
commented
- Add workflow for published change build validation
- Add a reworked version of https://github.com/Itiviti/gerrit-trigger-plugin-obsolete-2023-11-14/commit/a9538717b84348d91e5ed0c3eb03f34b9853c29f by updating UI (the way a user can configure the labels & voting values), refactored couple of classes & added fixed PR reviews from Add support for custom defined labels (verdict categories beside Verified & Code-Review) jenkinsci/gerrit-trigger-plugin#393
- Codestyle errors are still TODO (but not important now for our version of the plugin)
- for reuse in global context
- moved build status votes fields to VerdictCategory instead of having them as private fields in Config/GerritTrigger & mark their getters/setters deprecated - remove the entries for the removed status votes from jelly gerrit-trigger config & add them in VerdictCategory list instead (all the votes are set within VerdictCategory) - votes are now part of the VerdictCategory list (categories) so all deprecated methods are now searching through the list instead
- some methods are redundant (containing 'verified' name although a simpler form already exists) - extact default gerrit commands to static fields instead
- deprecate verified/code-review minimum vote calculation methods - create string based label methods for minimum voting calcs and aggregate all categories (verified & code-review included) into the templated command allowing them to be expanded later all together - remove redundant test (in which the scenario was testing the min. voting calculation for the Verified label by using its associated method instead of testing the scenario via getBuildCompletedCommand) - update tests to check for the newly added custom label
timotei
reviewed
Mar 29, 2024
| if (categories.isEmpty()) { | ||
| categories.add(new VerdictCategory("Code-Review", "Code Review")); | ||
| categories.add(new VerdictCategory("Verified", "Verified")); | ||
| categories.add(new VerdictCategory(Constants.CODE_REVIEW_LABEL, Constants.CODE_REVIEW_LABEL)); |
There was a problem hiding this comment.
This looks wrong? One is "Code-Review", and the other is "Code-Review"... Or we want to display the same value as the actual label? 🤔
| * Returns the value formatted as a placeholder. | ||
| * @return the value formatted as a placeholder. | ||
| */ | ||
| public String getPlaceholderValue() { |
There was a problem hiding this comment.
Not super clear where this is used, I can't find usages.
Comment on lines
+64
to
+69
| Integer buildStartedVote, | ||
| Integer buildSuccessfulVote, | ||
| Integer buildFailedVote, | ||
| Integer buildUnstableVote, | ||
| Integer buildNotBuiltVote, | ||
| Integer buildAbortedVote) { |
There was a problem hiding this comment.
Don't think "VerdictCategory" should be combined with the Vote, but rather have a separate class (which potentially composes this class to store the verdict for which the votes are for)
| if (result == Result.ABORTED) | ||
| return BuildStatus.ABORTED; | ||
|
|
||
| return BuildStatus.FAILED; |
There was a problem hiding this comment.
Sounds like NOT_REGISTERED should be here, no?
| for (Object jsonObject : (JSONArray)cat) { | ||
| categories.add(VerdictCategory.fromJSON((JSONObject) jsonObject)); | ||
| } | ||
| } else if (cat instanceof JSONObject) { |
There was a problem hiding this comment.
should probably throw here if none of the checks match? i.e., unknown type.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.