Added a 'lint' command #511
Conversation
| .setAuthor(johnDoe) | ||
| .setCommitter(johnDoe) | ||
| .setMessage("Test commit") | ||
| .setSign(false) // Required if you're locally setup for signed commits. |
There was a problem hiding this comment.
Locally, my gitconfig is setup for signed commits and that was causing this test to fail with a signing service is not available exception message. Using org.jenkinsci.plugins.gitclient.GitClient, it was not possible to configure it to disable signed requests. Switching to org.eclipse.jgit.api.Git allows this an lets the test pass for folk with setups like mine.
Co-authored-by: Jesse Glick <jglick@cloudbees.com>
Err...no I didn't 🤔 |
| JenkinsLauncher<?> launcher; | ||
| if (command instanceof RunJenkinsfileCommand) { | ||
| launcher = new JenkinsfileRunnerLauncher((RunJenkinsfileCommand) command); | ||
| } else if (command instanceof LintJenkinsfileCommand) { |
There was a problem hiding this comment.
Later some simplification will be needed for the Command => Launcher mapping. Fine for now
There was a problem hiding this comment.
Agreed. I did take a quick look at this. Thought it would be good if each JenkinsLauncherCommand had a factory method that allowed it to create the right type of JenkinsLauncher, but that would have introduced a circular dependency between the bootstrap and setup modules. Will leave it for now while it's still simple.
| @Override | ||
| protected int doLaunch() throws Exception { | ||
| // So that the payload code has all the access to the system | ||
| ACL.impersonate(ACL.SYSTEM); |
There was a problem hiding this comment.
This is the deprecated call, right? Better to do try-with-resources method so that we do not leak the context
There was a problem hiding this comment.
Can't do a try-with-resources with Java 8, so swapped to a try-finally block.
There was a problem hiding this comment.
Wait! What? Yes, you can do a try-with-resource at Java 8 🤦
My IDE is lying. One second, let me check that again.
There was a problem hiding this comment.
Ok. Changed it to a try-with-resources. Also took the liberty of changing JenkinsfileRunnerLauncher too, so that both files are consistent.
| try { | ||
| // Attempt to call the scriptToPipelineDef method of the Converter class. This is the same as what | ||
| // happens when a Jenkinsfile is POSTed to $JENKINS_URL/pipeline-model-converter/validate. | ||
| System.out.println("Linting..."); |
There was a problem hiding this comment.
Fine for now though I want to replace everything by loggers later
There was a problem hiding this comment.
CI fails because of the infra issue. The code looks good overall, thanks @pawoolley !
Added a
lintcommand, which is akin to when a Jenkinsfile is POSTed to$JENKINS_URL/pipeline-model-converter/validate.This has the benefits of being:
Running...
...vs. linting...
