Skip to content
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

Use BRANCH_NAME in environment as fallback #1288

Merged
merged 13 commits into from May 11, 2023

Conversation

martinnemec3
Copy link
Contributor

@martinnemec3 martinnemec3 commented May 17, 2022

Using ff only merging strategy in gitlab results in having same commit on a feature branch and on the main branch (there it appears after the merging is done). This PR tries to fix the situation when jenkins multibranch pipeline job is used to build both the feature branch and main branch. The problem is that status update request does not contain the "ref" parameter needed to set the status for branch correctly.

Fixes #1010.

@martinnemec3 martinnemec3 requested a review from a team as a code owner May 17, 2022 23:16
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR tries to fix the situation

Does this PR try to fix the situation, or do you know for sure that it fixes the situation? That is, are you running this PR in production and have you verified this PR fixes the problem? I am more of an interim maintainer than a maintainer, I don't use GitLab, and I have no way of testing this change. I would only feel comfortable merging and releasing this PR if you have tested it on a production Jenkins server.

@martinnemec3
Copy link
Contributor Author

martinnemec3 commented Jun 20, 2022

Does this PR try to fix the situation, or do you know for sure that it fixes the situation?

It actually fixes the situation.

That is, are you running this PR in production and have you verified this PR fixes the problem?

The problem is not hard to reproduce so I just tested the fix in my local (docker) test environment (set up by following https://github.com/jenkinsci/gitlab-plugin/blob/master/src/docker/README.md).
I do not run the code anywhere in production.

@basil
Copy link
Member

basil commented Jun 21, 2022

Any chance you could download the incremental build from https://repo.jenkins-ci.org/incrementals/org/jenkins-ci/plugins/gitlab-plugin/1.5.33-rc1406.50cf09d3a_6fd/ and do a functional test on a production server? For instructions on how to install a custom plugin build, see: https://www.jenkins.io/doc/book/managing/plugins/#advanced-installation

@basil
Copy link
Member

basil commented Jun 21, 2022

Sorry to leave you halfway through review, but I recently removed myself as the maintainer of this plugin in jenkins-infra/repository-permissions-updater#2605, so I will not be able to merge this PR. You can consider adopting the plugin to merge and release the PR. The "Contributing to Open Source" workshop from DevOps World 2021 is a useful starting point for new maintainers. That document includes links to a five part video series that illustrates many of the steps. If the plugin is crucial to your work, you may want to ask your employer to support your work efforts by allowing you to adopt the plugin.

@MarkEWaite MarkEWaite added the rfe label Nov 22, 2022
@krisstern krisstern changed the title use BRANCH_NAME from environment as fallback for commit status ref Use BRANCH_NAME from environment as fallback for commit status ref Jan 31, 2023
@krisstern
Copy link
Member

@martinnemec3 Thanks for the PR! The solution looks intuitive enough for me.

@krisstern krisstern self-assigned this Jan 31, 2023
@krisstern
Copy link
Member

An ideal situation is if someone can test this out on a production server...

@krisstern krisstern added bug For changelog: Minor bug. Will be listed after features and removed rfe labels May 10, 2023
@krisstern krisstern changed the title Use BRANCH_NAME from environment as fallback for commit status ref Use BRANCH_NAME in environment as fallback May 10, 2023
@krisstern krisstern merged commit dd23f4c into jenkinsci:master May 11, 2023
15 checks passed
@krisstern
Copy link
Member

Thanks @martinnemec3!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Minor bug. Will be listed after features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gitlabCommitStatus pushing status to wrong branch
4 participants