git-plugin
git-plugin copied to clipboard
jenkinsci
[JENKINS-42971] GitSCMFileSystem consumes environment and expands branch name
GitSCMFileSystem now implements new SCM Builder APi that receives the Run,?> that can is used to expand branch name with environment variables.
Hmm i wonder what the "/* Unreliable on Windows and not a platform specific test */" means in the failing test...
Hmm i wonder what the "/* Unreliable on Windows and not a platform specific test */" means in the failing test...
:) It means that the test has been unreliable in the past on Windows and the maintainers did not have time to understand why it was unreliable on Windows. History of that is captured in the commit messages in case you want to see the sordid details.
:) It means that the test has been unreliable in the past on Windows and the maintainers did not have time to understand why it was unreliable on Windows. History of that is captured in the commit messages in case you want to see the sordid details.
Hmm so I understand that I should just ammend commit and run it again as there might be a chance I didnt break it. I was not able to debug the test in my Idea for some strange reason, so I cannot tell if this test was affected by my changes
Edit: Ok I was able to run it in debugger and it was probabbly not related to my changes :)
I did a small refactor of the calculation of the headName. I dont know what is the best practice in this project, I made a dedicated public static method, so that it could be tested standalone. Do you prefer a utility class or can we leave it as is ?
I need it in incrementals, so I can make an integration test in workflow-cps, so i converted it to to PR (removed draft)
I need it in incrementals, so I can make an integration test in workflow-cps, so i converted it to to PR (removed draft)
Incrementals are published from draft PRs.
https://github.com/jenkinsci/git-plugin/pull/898#discussion_r448561267 should be investigated. Also that PR touches GitTelescope (whatever that is). Need to examine prior art.
I apolagize, I had my linter reconfigured for wrong project, now it should look better
@MartinKosicky by the way when you force-push inside a PR, it breaks the ability of GitHub to display incremental diffs to reviewers. Generally I advise simply pushing commits to the branch, and leaving it to up the maintainer whether to squash-merge or true-merge at the end. (Some projects rebase-merge and so the list of commits inside the PR branch is actually critical, but this is rare in Jenkins components.)
ply pushing commits to the branch, and leaving it to up the maintainer whether to squash-merge or true-merge at the end. (Some projects rebase-merge and so the list of commits inside the PR branch is actually critical, but this is rare in Jenkins components.)
I'm sorry, I thought this was the prefered way (I read it somewhere on the internet that it's prefered in github's PRs, as this is my first contribution to a github project). I will do it only incrementally from now. Also I did some ammends because the builder was broken, in that case should I make a dummy commit to retrigger build?
I advise simply pushing commits to the branch, and leaving it to up the maintainer whether to squash-merge or true-merge at the end.
Considering the number of times that I've made mistakes visible in the history of the master branch of the git plugin, I don't think a history of mistakes and missteps by others will feel at all out of place in the git plugin. If the submitter asks for a squash merge or if the merge history is especially "long and dirty", then I'll squash merge.
I read it somewhere on the internet that it's prefered in github's PRs
I am afraid every project has its own conventions, and preferences are certainly not consistent inside @jenkinsci which is quite decentralized.
I did some ammends because the builder was broken, in that case should I make a dummy commit to retrigger build?
That is one way to trigger a fresh build of your own PR, yes. (People with write access to the repository can Re-run from the Checks tab. Fewer people have permission to directly Rebuild or Replay on ci.jenkins.io.)
@jglick I would like to ask what is the flow here, for example when the https://github.com/jenkinsci/scm-api-plugin/pull/160 will be ok and merged, should we wait until the scm-api-plugin new version will be released (i dont know what is the time interval here, or release schedule) before marking this PR as ready for review?
scm-api is set up for CD so release occurs automatically shortly after merging PRs with applicable labels. Ditto workflow-cps. I cannot speak for git.
should we wait until the scm-api-plugin new version will be released (i don't know what is the time interval here, or release schedule) before marking this PR as ready for review?
You can mark the PR as ready for review as soon as you feel it is ready. No need to wait for releases of the dependent plugins so long as your pull request refers to an incremental build of the dependent plugins. If your pull request refers to incremental builds of the dependent plugins, then I can perform testing with an install of those dependent plugins along with a build of your pull request.
#898 (comment) should be investigated. Also that PR touches
GitTelescope(whatever that is). Need to examine prior art.
Yes i saw that PR, and that is why I moved expansion at start, and also added tests to make sure that if BRANCH is */master it doesnt get converted to */*/master
Reason why i started this PR is because that one is opened for 2 years, and since we have 1GB monorepo, it is very expensive to pull jenkinsfile whem we use parameters (the BRANCH we use for pipeline and in pipeline on various nodes we check out from same branch as is the pipeline) . We also hqve around 50 jobs so you can imagine the ammount of data
Ok so If I understand correctly, since we change the version from incremental to that which was produced by CI/CD is that correct and can this be merged or are we going to wait for that BOM (which I know about only theoretically) ?
Ok so If I understand correctly, since we change the version from incremental to that which was produced by CI/CD is that correct and can this be merged or are we going to wait for that BOM (which I know about only theoretically) ?
Since the scm-api that is now referenced in the pom is a released version of scm-api, this change could be merged. No need to wait for scm api to appear in the bom.
The TODO is to remind us that once the version is included in the bom, then the version entry for scm-api can be removed.
Ok so If I understand correctly, since we change the version from incremental to that which was produced by CI/CD is that correct and can this be merged or are we going to wait for that BOM (which I know about only theoretically) ?
Since the scm-api that is now referenced in the pom is a released version of scm-api, this change could be merged. No need to wait for scm api to appear in the bom.
The
TODOis to remind us that once the version is included in the bom, then theversionentry for scm-api can be removed.
Hmm oki so i guess ill wait for some additional feedback , because i am not able to merge this PR
Hmm oki so i guess ill wait for some additional feedback , because i am not able to merge this PR
I still need to review the PR. I have others in my review queue ahead of this PR. I will be unavailable next week while I take vacation. It will be at least two weeks before I can review this PR. If other maintainers would like to review it, they are welcome to do so.
The next release of the master branch of the git plugin will not happen for at least two weeks. I don't want to risk releasing a new version of the plugin shortly before I disappear for a week of vacation. It is better for the project if I am available for the week after a git plugin release in case there are surprises or other problems.
Hmm oki so i guess ill wait for some additional feedback , because i am not able to merge this PR
I still need to review the PR. I have others in my review queue ahead of this PR. I will be unavailable next week while I take vacation. It will be at least two weeks before I can review this PR. If other maintainers would like to review it, they are welcome to do so.
The next release of the master branch of the git plugin will not happen for at least two weeks. I don't want to risk releasing a new version of the plugin shortly before I disappear for a week of vacation. It is better for the project if I am available for the week after a git plugin release in case there are surprises or other problems.
Ah oki then, i dont mean to rush anyone. Oki so I will check in 2 weeks or so, and enjoy vacation:)
Just wanted to let you know that we took the incremental plugin builds for now, and put in our production jenkins, and configured all jobs to use lw checkout now, it works perfectly
Hello, this is just a friendly reminder as I am not sure if it wasn't forgotten :)
Hello, this is just a friendly reminder as I am not sure if it wasn't forgotten :)
Thanks for the reminder. I've returned from a week of vacation, will be in the office for three days, then will be out of the office for four days. I still have other reviews that precede this review in my queue, so it will be at least 2 weeks before I can review it.
Other maintainers are welcome to review it if they wish.
Hello pinging again as this PR is opened too long, can we either finish it up somehow or probabbly add some improvements? @MarkEWaite or @dwnusbaum or @car-roll ?
Thanks for the ping. I'll be busy all this weekend preparing for DevOps World and then busy all week at DevOps World. I may be able to start the review a week or two after DevOps World
Can we retrigger? If i force push (just to trigger build) the approvals will disapear and the test that failed seems to be some kind of unreliable test (should not affected by my change)