-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 all artifacts to maintainers and issue lists when wildcard is used #4108
Conversation
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.
LGTM, can tweak after easily if needed
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.
Should probably be documented in the readme?
if (lastPathElement.endsWith("-releaseblocker") || lastPathElement.endsWith("-releaseblock")) { | ||
LOGGER.log(Level.INFO, "Release blocked for artifact ID: " + artifactId) |
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.
Rather than add special handling for a weakly adhered to naming convention, if we add code for this case, it should just be a new attribute.
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.
Done in #4127
|
||
if (maintainersByComponent.containsKey(key)) { | ||
LOGGER.log(Level.WARNING, "Duplicate maintainers entry for component: " + key) | ||
if (lastPathElement.contains("*")) { |
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.
Seems unnecessarily complicated, because extraNames
is expected to only exist with wildcard in the first place?
@@ -358,6 +364,16 @@ class ArtifactoryPermissionsUpdater { | |||
new File('checks-details.txt').text = details | |||
} | |||
|
|||
private static void addMaintainers(HashMap<String, List<String>> maintainersByComponent, String key, Definition definition) { | |||
if (maintainersByComponent.containsKey(key)) { | |||
LOGGER.log(Level.WARNING, "Duplicate maintainers entry for component: " + key) |
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.
Interestingly, we have some of these in the log. Anyone want to look into this?
Needs to be tested first. For some plugins that moved groupIds a few times we had to remove the old ones due to permission target length limitations at the time. |
Fixes #2911
Allows each component to define extra artifact names, those then inherit the same maintainers and issue tracker(s) as the parent component.
A more straightforward approach might be to just define
paths
explicitly for each component without using *, but since the last element of path sometimes doesn't match the component name and the component name takes precedence, I decided to add a new field to Definition for defining the extra components.