Hello, First of all, I'd like to apologize in advance for the rant I'm about to post. This all started as a discussion with my colleagues, which lasted way too long, on a simple method name. I'd appreciate it if you'd just tell us we're crazy and should just make a pull request.
We use the Git Client Plugin in one of our plugins and are lacking a couple of options, which we're adding at the moment. I'm currently working on adding the --no-commit option for the MergeCommand and wanted some input on naming/default behaviour. I added a 'setCommit' method to MergeCommand, accepting a boolean which defaults to true. For JGit, I call JGit's own 'setCommit', passing in the boolean. Looking at JGit's implementation, I realized that this changes the default behaviour of the Git Client Plugin (as did the implementations of the squash/message options, by the way). If JGit's 'setCommit' isn't called, it will look at the merge configuration of the repository. So simply by calling the JGit methods in our implementation, we're already enforcing our own defaults. Not to make a fuss about it, just wanted to bring that up. For the CLI implementation, I only add '--no-commit' when setCommit is set to false. If setCommit is said to true, I do nothing. This is in line with the implementations of the other options. I didn't want to explicitly add '--commit' when it was set to true, because that would change the default behaviour of the plugin. That's the way I like it, but it's not in line way the JGit implementation, where we always set either --commit or --no-commit. I wouldn't mind hearing your opinions on the matter. Kind regards, Thierry -- You received this message because you are subscribed to the Google Groups "Jenkins Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/477f41cf-235a-4e8e-beeb-15edaaa4821e%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
