[JENKINS-52304] Tweaks for JEP-201 #4
Conversation
jglick
requested review from
carlossg,
ndeloof,
batmat,
oleg-nenashev and
kuisathaverat
| @@ -55,5 +55,10 @@ | |||
| <artifactId>aws-credentials</artifactId> | |||
| <version>1.23</version> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>org.jenkins-ci.plugins</groupId> | |||
| <artifactId>structs</artifactId> | |||
There was a problem hiding this comment.
Previously picked up @Symbol from jenkins-core, which is dangerous for class loading reasons IIRC.
|
|
||
| /** | ||
| * @author Carlos Sanchez | ||
| * @since | ||
| * | ||
| */ | ||
| @Symbol("aws") |
There was a problem hiding this comment.
Inferred by chomping off pieces of the Class.simpleName, but best to be explicit.
| @@ -64,7 +64,8 @@ | |||
| /** | |||
| * Store the AWS configuration to save it on a separate file | |||
| */ | |||
| @Extension @Symbol("credentials") | |||
| @Symbol("awsCredentials") | |||
There was a problem hiding this comment.
While credentials actually looks a bit nicer in the content of JEP-201, it is illegal for @Symbol generally as the ExtensionList here is GlobalConfiguration, not AbstractAwsGlobalConfiguration, so that would (probably) not be unique.
There was a problem hiding this comment.
From JCasC point of view the symbol scope is the GlobalConfiguration category, so "credentials" is fine. But all yours.
There was a problem hiding this comment.
The issue is that it is ambiguous what a symbol scope will be. The Javadoc only promises that
symbol names are meant to be only unique within a specific extension point
which is fairly vague. Currently configuration-as-code does restrict the scope by category (I suppose in GlobalConfigurationCategoryConfigurator.describe), meaning it actually uses a different interpretation of the specification (since a GlobalConfigurationCategory is a classification but not an “extension point”). Other tools hewing more closely to the spec might call something like
SymbolLookup.find(GlobalConfiguration.class, symbol)since that type is where the ExtensionPoint is declared.
Used by jenkinsci/artifact-manager-s3-plugin#72.