-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: provide COMMIT_MESSAGE as an env var #796
Conversation
I think so ~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable, though I think you may want to name the variable GIT_COMMIT_TITLE
rather than GIT_COMMIT_MESSAGE
based on the description in the git commit manual page where it says:
The text up to the first blank line in a commit message is treated as the commit title, and that title is used throughout Git. (emphasis added)
That would allow a future addition to deliver GIT_COMMIT_MESSAGE
as the full message rather than only the first line of the message.
Also needs one or more tests to confirm that it is well-behaved.
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't add extra white space in a change. The white space is already messy enough in the plugin without adding duplicated empty lines
I've updated with the documentation bits that seem to be needed. I've also added a test, but the test doesn't actually pass. Spent some time trying to debug it, but may need to take a different approach. |
* rename the env var to be aligned with the git description * test isn't working yet as expected, still need more investigation into why Signed-off-by: Jeff Knurek <[email protected]>
a675ff4
to
536b177
Compare
@jeff-knurek I don't understand why are you asserting String with Env object. Maybe you wanted |
Signed-off-by: Jeff Knurek <[email protected]>
@otradovec you're totally right. I had pushed some in-progress code from while I was debugging the test. I've now pushed what I expect to work (but doesn't) |
What about using GIT_COMMIT instead? I don't see GIT_COMMIT_TITLE here https://javadoc.jenkins.io/plugin/git/hudson/plugins/git/GitSCM.html |
Btw this: https://javadoc.jenkins-ci.org/hudson/model/Run.html#getEnvVars-- |
Your tip about |
Hi @MarkEWaite, is there anything remaining for this PR? It's a useful feature. |
I never got the unit tests to work. I think that's pretty critical to get this out. I'm not a Java/Jenkins developer though, so I think I've run out of things to try. |
@@ -3064,6 +3064,24 @@ public void testCommitMessageIsPrintedToLogs() throws Exception { | |||
assertThat(values, hasItem("Commit message: \"test commit\"")); | |||
} | |||
|
|||
@Test | |||
public void testCommitMessageIsEnvVar() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to this I was trying to set environment variable for https://issues.jenkins.io/browse/JENKINS-38699. Does this test case run correctly in the current state?
@@ -1352,6 +1353,9 @@ public void checkout(Run<?, ?> build, Launcher launcher, FilePath workspace, Tas | |||
|
|||
// Needs to be after the checkout so that revToBuild is in the workspace | |||
try { | |||
String shortMessage = getCommitMessage(listener, git, revToBuild); | |||
listener.getLogger().println("Commit message: \"" + shortMessage + "\""); | |||
environment.put(GIT_COMMIT_TITLE, shortMessage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MarkEWaite In this(https://github.com/jenkinsci/git-plugin/pull/1049/files#r583842074) pull request you mentioned with this enviroment variable is set only for the process.
I want to understand what is the scope of adding a variable to EnvVars is? Since it is being added in git plugin would it be available to just the checkout step or it will be available in subsequent steps like build etc. ? From the bug description I believe we want to use this env variable to check if we want to run the build step based on the commit message. So would this env variable be available in the pipeline script in the build step with this ?
Closing in favor of #1074 |
JENKINS-52490 - Plugin should provide GIT_COMMIT_MESSAGE as Environment Variable
The commit message should be available as an env var.
Checklist
Types of changes