Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-37391

Add @Symbol("git") (and "jgit") to Git Client's ToolDescriptors

    XMLWordPrintable

    Details

    • Similar Issues:

      Attachments

        Issue Links

          Activity

          Show
          abayer Andrew Bayer added a comment - PR up - https://github.com/jenkinsci/git-client-plugin/pull/215
          Hide
          markewaite Mark Waite added a comment -

          I don't object to this, but I don't understand who benefits by it, how they benefit, or the impact of the addition.

          Some sample questions:

          • Who does this help?
          • How does it help them?
          • What are the risks from the change?
          • Where are the allowed `@Symbol` values described and how are they used? For example, today there is a "Default" implementation (which is command line git) and an optional "jgit" implementation which the administrator can add, then can use from within jobs
          Show
          markewaite Mark Waite added a comment - I don't object to this, but I don't understand who benefits by it, how they benefit, or the impact of the addition. Some sample questions: Who does this help? How does it help them? What are the risks from the change? Where are the allowed `@Symbol` values described and how are they used? For example, today there is a "Default" implementation (which is command line git) and an optional "jgit" implementation which the administrator can add, then can use from within jobs
          Hide
          abayer Andrew Bayer added a comment -

          The @Symbol annotation's reasoning and behavior is described over at the structs plugin wiki page. The allowed values are, well, any string, scoped to the ExtensionPoint in question. Right now, the only ToolDescriptor with the @Symbol annotation are JDK and MavenInstallation in core. I'd like to add this to more tools to do a couple things:

          • Simplify the tool step in Pipeline by allowing you to specify the tool type without needing to give the ToolDescriptor class name - i.e., right now, if you want to be specific and not just specify the install name, you need to do tool name: 'maven3.3.9', type: 'hudson.tasks.Maven$MavenInstallation' (yeah, in this case, there's no ambiguity, but you know what I'm getting at. =) ), and with the @Symbol on all ToolDescriptor, I'll be able to change the tool step to allow tool name: 'maven3.3.9', type: 'maven', etc...
          • I've got a project that'll be coming to the jenkinsci org in the next couple weeks that provides a more config-like way to define your Pipelines, and having the @Symbol on tools will make my tools section much, much cleaner.
          • and in general, there's been a trend towards trying to get @Symbol on Extension uses to open up more DSL possibilities down the road and get rid of more and more of the ugly [$class: 'Foo', arg1: ...] syntax needed for some things now.

          There are basically no risks to this at all. As I said, the scoping for the @Symbol values is by ExtensionPoint, so we can have multiple uses of the same name as long as they're on different ExtensionPoint - see the build step descriptor and the tool descriptor for Maven, for example. All this does is add some discoverable metadata to the descriptors.

          As I mentioned over on the PR, I'm completely flexible on the naming - I just went with my default mentality, where GitTool proper would line up with git as a name, and JGitTool would get jgit as a name.

          Show
          abayer Andrew Bayer added a comment - The @Symbol annotation's reasoning and behavior is described over at the structs plugin wiki page . The allowed values are, well, any string, scoped to the ExtensionPoint in question. Right now, the only ToolDescriptor with the @Symbol annotation are JDK and MavenInstallation in core. I'd like to add this to more tools to do a couple things: Simplify the tool step in Pipeline by allowing you to specify the tool type without needing to give the ToolDescriptor class name - i.e., right now, if you want to be specific and not just specify the install name, you need to do tool name: 'maven3.3.9', type: 'hudson.tasks.Maven$MavenInstallation' (yeah, in this case, there's no ambiguity, but you know what I'm getting at. =) ), and with the @Symbol on all ToolDescriptor , I'll be able to change the tool step to allow tool name: 'maven3.3.9', type: 'maven' , etc... I've got a project that'll be coming to the jenkinsci org in the next couple weeks that provides a more config-like way to define your Pipelines, and having the @Symbol on tools will make my tools section much, much cleaner. and in general, there's been a trend towards trying to get @Symbol on Extension uses to open up more DSL possibilities down the road and get rid of more and more of the ugly [$class: 'Foo', arg1: ...] syntax needed for some things now. There are basically no risks to this at all. As I said, the scoping for the @Symbol values is by ExtensionPoint , so we can have multiple uses of the same name as long as they're on different ExtensionPoint - see the build step descriptor and the tool descriptor for Maven , for example. All this does is add some discoverable metadata to the descriptors. As I mentioned over on the PR, I'm completely flexible on the naming - I just went with my default mentality, where GitTool proper would line up with git as a name, and JGitTool would get jgit as a name.
          Hide
          markewaite Mark Waite added a comment -

          Thanks for the additional info. Given that info, I'm accepting the pull request "as is", with "git" and "jgit". Those are the most "natural" names for me since those are the argument values for the `Git.with(String)` call, and are the common terms used to refer to the two different implementations throughout the source code.

          Show
          markewaite Mark Waite added a comment - Thanks for the additional info. Given that info, I'm accepting the pull request "as is", with "git" and "jgit". Those are the most "natural" names for me since those are the argument values for the `Git.with(String)` call, and are the common terms used to refer to the two different implementations throughout the source code.
          Hide
          abayer Andrew Bayer added a comment -

          Merged - thanks, Mark Waite!

          Show
          abayer Andrew Bayer added a comment - Merged - thanks, Mark Waite !

            People

            Assignee:
            abayer Andrew Bayer
            Reporter:
            abayer Andrew Bayer
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: