[JENKINS-62130] Add Symbols to extenders of SCMSourceTrait and friends#886
[JENKINS-62130] Add Symbols to extenders of SCMSourceTrait and friends#886kshultzCB wants to merge 101 commits intojenkinsci:masterfrom
Conversation
Pipeline syntax creates the simpler form once symbols are defined. Randomized testing checks the various forms of checkout scm extensions.
Don't test in the git plugin, that's the responsibility of the plugin that provides the implementation.
…yBrowserDescriptor
…sitoryBrowserDescriptor
Understood that some of the options require more configuration than is available in these small tests.
The symbol should not collide with the "gitBlit" symbol defined in the gitblit plugin. Good case to test by installing the gitblit plugin and confirming that pipeline syntax shows the correct value for the gitblit browser.
fcojfernandez
left a comment
There was a problem hiding this comment.
The PR seems to be ok, but in those cases where the symbol changes the expected key word for CasC configuration, the yaml files have to be updated. For example, I would expect that https://github.com/jenkinsci/git-plugin/blob/master/src/test/resources/jenkins/plugins/git/browsers-casc.yaml should have to be updated. (See my comment in gitLabBrowser)
And for those cases we should add a warning in the README file.
| } | ||
|
|
||
| @Extension | ||
| @Symbol("gitLabBrowser") |
There was a problem hiding this comment.
this is a change that would impact in the yaml configuration for CasC. So far, the exportation / configuration would have expected "gitLab":
But with this change, the key word would be "gitLabBrowser". The former yaml configuration files have to be changed.
There was a problem hiding this comment.
CasC is able to recognize several values for the same descriptor, but it should be verified both of them work now.
There was a problem hiding this comment.
good to know! I thought CasC would only take into account one of them. That explains why browsers-casc.yaml didn't have to be updated
There was a problem hiding this comment.
This specific example is more complicated than you might expect (and thus even more interesting to assure that retain compatibility).
The GitLab plugin provides a repository browser with class name GitLabBrowser. It takes no arguments and appears in the pick list as "GitLab".
The Git plugin provides a repository browser with a class name GitLab. It takes one mandatory argument and an optional argument and appears in the pick list as "gitlab".
I assumed (possibly incorrectly) that the JCasC symbol "gitLabBrowser" was derived from the repository browser provided by the GitLab plugin, not from the repository browser provided by the Git plugin.
There was a problem hiding this comment.
CasC is able to recognize several values for the same descriptor, but it should be verified both of them work now.
That gives me hope. We might be able to define a symbol for compatibility with existing JCasC YAML files and also define a shorter or clearer symbol as the preferred symbol.
What types of things would we need to test to confirm that works as expected?
- JCasC config file for git plugin 4.2.2 (current release) loads correctly into new version
- JCasC config file for git plugin 4.3 (next release) exports correctly from JCasC page
- JCasC config file for git plugin 4.3 (next release) loads correctly
- Pipeline definitions from git plugin 4.2.2 (current release) load correctly in new version
- Pipeline syntax editor for git plugin 4.3 (next release)
There was a problem hiding this comment.
What types of things would we need to test to confirm that works as expected?
Regarding the JCasC testing, that's what have to be done. Regarding the pipeline, I'm not sure
I assumed (possibly incorrectly) that the JCasC symbol "gitLabBrowser" was derived from the repository browser provided by the GitLab plugin, not from the repository browser provided by the Git plugin.
The BrowsersJCasCCompatibilityTest (via the browsers-casc.yaml) is testing the former gitlab key word and it's working. In a production environment where we have installed the gitlab-plugin and the git-plugin, if we can have both repository browsers, we have to check which key word is exported for each case. If they are not the same (gitLabBrowser vs gitlab), there should be no problem, since the configurators will be able to distinguish between them. But if they are the same with this new Symbol, then I foresee we'd get an error. I guess this check must be done with the current release and with this PR merged
There was a problem hiding this comment.
CasC as of a few versions now considers all symbols on import and on export will show the first one in the array
Assumed that Jenkins will disambiguate those based on the context where they are used.
|
I'm closing this pull request as idle. I will continue to track the work in my https://github.com/MarkEWaite/git-plugin/tree/add-symbols branch and will open a new pull request as time and testing capacity allows. |
JENKINS-62130 - Add
@Symbolannotations to extenders ofscm-api:SCMSourceTraitChecklist
Types of changes
Further comments
The
@Symbolannotation can make it easier for things outside of the Git plugin, such as CasC and Job DSL. The Git plugin has@Symbolannotations for many of its classes which extendscm.api.trait.SCMSourceTrait- this PR adds similar@Symbolsto remaining classes.Additionally, because
GitBrowserSCMSourceTraithas aDataBoundConstructorwhich takes aGitRepositoryBrowseras an argument, I've added@Symbolsto classes which extendGitRepositoryBrowseras well.