-
-
Notifications
You must be signed in to change notification settings - Fork 279
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
Add people with merge rights #366
Comments
Help would be greatly appreciated, let me know if anyone has capacity to become the member of the "core team" :-) |
Maybe you should add something to the readme and/or the plugin wiki that you're looking for contributors? |
Thanks for offering your help, @erikhakansson! It would be awesome if you could help out with getting #365 to a mergeable state as that issue seems to affect a bunch of people. |
I'll start looking at that. However, not all tests go through and they shouldn't be related to the proxy stuff, so I wonder if the tests are working for you in the master branch? |
The master branch is also broken. The problem is that the tests will pull the latest version of the plugins that the build-monitor plugins can integrate with (using the install manager) but those plugins are not compatible anymore with the jenkins core that is being used. In my opinion this is two different problems:
|
@dcendents ok, I see! |
Look at this: https://wiki.jenkins.io/display/JENKINS/Choosing+Jenkins+version+to+build+against I suppose the best thing would be to keep a low version (2.19.4), test against that version but also find a way to test against the latest LTS as well (maven profiles?). |
Okay thanks. After that I'll see what has to be done to run on later Jenkins versions |
The more I think about it maybe we should let the install manager as it is and install the latest version of the plugins. Since 2.46, jenkins have introduced different update sites to make sure we get only plugins compatible with a given core version. e.g.:
So it might be a good idea to keep the current behavior as it would test what the users experience. It means the tests might break when a dependent plugin is updated but at least we are testing the reality. So we could update the core version to 2.46.3 and find a way to test every LTS release since then. If a new LTS release breaks the build, we either try to be backwards compatible or we increase the core version and it means only people using the latest LTS will get the new features. People using and older version will still be able to install an older version of the build monitor that was compatible with that specific LTS version. e.g.:
If I use jenkins 2.89.3, the update site would let me install version 2.0 which is fine for me. To get new features I need to update jenkins to 2.107.1 |
Yeah that sound's like a plan. And I was almost done with fixing the master branch the old way 😏 |
I'll still do it twofold, though, although the steps will be
|
Status update: I've bumped the Jenkins version to 2.46.3, the org.jenkins-ci.plugins:plugin parent to 3.8 and the jenkins-test-harness to 2.38. The bumped parent pom includes a lot of new Maven Enforcer rules and findbugs rules that I had to solve, so after wrestling with conflicting dependencies, and bumping some here and there and simply overriding some rules I've gotten everything to compile. build-monitor-plugin builds without issue now, while pretty much every test in build-monitor-acceptance fails. So the next step is to solve that. I haven't merged in #365 yet, though, as I want as much as possible working before I do that. |
Another status update: I still haven't run into any issues regarding #364 but I think one test might have that issue (Badges) so I'll leave that for last. Hopefully I'll have a PR soon. I hope to be able to work on it the coming evenings. It would be great if both @jan-molak and @dcendents could have a look at it then. |
@jan-molak and @dcendents please have a look at #369. Note that I have ONLY bumped the Jenkins version in that. I haven't merged the #365 PR, nor have I made forward-testing possible. I think that should be a combined second step after this is merged. |
@erikhakansson sounds like a plan! I've merged #369, happy to help with the subsequent PRs. Jan |
Thanks @erikhakansson , I was actually logging in to check the work like you asked, I'm glad @jan-molak had time to do it and merge it. |
Thanks guys! I'm on to step two, which is "future"-testing with the latest LTS. There were some changes in AbstractBuild (on the Jenkins side) that required some additional mocks for the unit tests to work, but it was easily solved with PowerMockito (to maintain class compatibility between 2.46 and newer). The integration tests are going fine, except the Badges one. That badges don't work was expected since that's pretty much the core of #364. However, I'm having some difficulties applying the fix while still retaining compatibility with older Jenkins versions. But I'm working on it! |
I've solved compatibility with both 2.46.3 and 2.107.2, but I managed to broke a couple of tests in the process so I'm working on solving them. However, I'm having some difficulties in automating testing both. When manually setting ${jenkins.version} it works. When adding executions to run both in different phases, using one ${jenkins.version} property and one ${jenkins.lts.version} property I get weird errors in Jenkins. I think it's related to the Jenkins Test Harness forcing the ${jenkins.version} on some dependencies. For now I added a profile -Plts-test that changes ${jenkins.version} to 2.107.2 and that way it works, but it would be far prettier to have it as part of the normal mvn clean verify process. |
I was thinking about the problem and I suppose it cannot be done easily with a single maven module. To run both sets of tests I think you will need to split them in different modules.
It shouldn't break the builds with cloudbees' jenkins. Otherwise we could start different jenkins instances (using docker) and configure a selenium grid to run the tests against both of them at the same time. But it might be harder to configure that in a free publicly available build server (cloudbees, travis, etc). |
Well it seems the "standard" way of testing plugins against other jenkins version is to use the plugin-compat-tester: https://github.com/jenkinsci/plugin-compat-tester The documentation is a bit poor but they have an example here under This means the tests would need to be run manually though. |
Unless I misunderstand, plugin-compat-tester isn't really for automated testing, right? I went with your first suggestion, splitting upp acceptance into two modules. PR #370 is waiting for review 😄 @jan-molak I added some comments for you where I was unsure of stuff. |
#370 is updated. I noticed that an old version of maven-enforcer-plugin was hardcoded in one of the submodules so I removed that to enable inheritance from the parent pom. I haven't had any issues locally but it could be related to your build troubles. |
Ping @jan-molak had any time to check the PR? I'm running it locally at work here (Well actually, I'm running #293 , but I've rebased that on #370) and it works fine. |
Also, I have a suggestion with dealing with the backlog of PRs. Since a lot of them, if not most, need to be rebased on the Jenkins 2 functionality, you could close all of them with a message like "Major changes have been made to Build Monitor Plugin. Therefore I'm closing all older PR's. Please rebase you PR against master and reopen pull request if still relevant". Or something. |
#370 builds fine for me as well. I had some small improvements I wanted to submit, I'll rebase on that and add some acceptance tests. |
It's looking good, thanks for your help and support, gents! I've just merged in #370 and it's building on Jenkins as we speak. |
Thanks! I've added #371 to hopefully solve the issue you had. |
Thanks! The build has just succeeded on Jenkins, so you can take the |
Great news! Thanks! 😄 I've also rebased #293 which was my initial motivation for getting involved 😄 I'd be very happy if you could have a look at that! |
Any news, @jan-molak ? Both on a release of the current state and on #293 |
Hi @erikhakansson! I've released the last build and should have some time over the next week or two to look into #293. Thanks for your efforts! |
Great, thanks! |
Had time yet? 😄 |
Hey guys, apologies, I've been snowed under. I'll set up a gitter channel for us this evening so that we can discuss giving people the merge rights and so on. Thanks again for offering your help and support! JAn |
That sounds great! I think that some merge and release rights would be the best course of action to avoid stagnation! |
Nothing has happened with this plugin for a long time despite some issues. If @jan-molak is too busy, maybe more people should be added to the project to offload?
The text was updated successfully, but these errors were encountered: