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

Require Java 11 and Jenkins 2.361.4 or newer #96

Merged

Conversation

VignanMudedla
Copy link
Contributor

@VignanMudedla VignanMudedla commented Apr 12, 2023

  1. updated parent pom to 4.59(allow compilation with java 11)
  2. updated base Jenkins version to 2.361.4
  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did

-updated parent pom to 4.59
-updated base jenkins version to 2.362.4
@VignanMudedla VignanMudedla requested a review from a team as a code owner April 12, 2023 18:40
@VignanMudedla
Copy link
Contributor Author

Hey @MarkEWaite! Can you help we with the windows-8 build fail.
I'm new to Open-source and this is my first PR. I Started contributing to Jenkins by watching " modernising the Jenkins Plugin tutorial"

I'll be more than happy if any one could help me with this

@MarkEWaite
Copy link
Contributor

It needs two more steps:

  1. Update the Jenkinsfile to replace Java 8 with Java 11 and Java 17 as described in "Add a Jenkinsfile"
  2. Replay the ci.jenkins.io job with the updated Jenkinsfile (needs a ci.jenkins.io admin like me to do that)

@VignanMudedla
Copy link
Contributor Author

Hey @MarkEWaite is this good?

Screenshot 2023-04-13 at 8 49 32 PM

can I add another new configuration for windows-17?

@MarkEWaite
Copy link
Contributor

MarkEWaite commented Apr 13, 2023

Hey @MarkEWaite is this good?

Screenshot 2023-04-13 at 8 49 32 PM

can I add another new configuration for windows-17?

Because the test configurations incur a cost, we increase test configurations only when we believe that the additional configuration will give useful information for the many times that it will be run (each pull request runs the Pipeline, master branch runs the Pipeline, etc.)

The "Improve a plugin" tutorial chooses only two configurations because that felt like the best balance between information and cost. One of the configurations is Linux and one is Windows because Windows and Linux have some very important differences. One of the configurations is Java 11 and one is Java 17 because there are some interesting differences between those two. So long as we touch each of those key differences in at least one configuration, that gives us most of the benefit without the cost of testing all combinations of the configurations. We're using pairwise testing in the Jenkinsfile to cover the most important cases without needing to perform exhaustive testing of all combinations

Short answer, I recommend against adding a separate test of Java 17 with Windows so long as Windows is tested in at least one configuration and Java 17 is tested in at least one configuration.

@VignanMudedla
Copy link
Contributor Author

Hey @MarkEWaite I've updated Jenkinsfile to java 11 and java 17 but Jenkins CI is yet building for java 8. build failed again.
I also updated Jenkins version for linux-17 to 2.361.4
https://ci.jenkins.io/blue/organizations/jenkins/Plugins%2Fjavadoc-plugin/detail/PR-96/3/pipeline/

sorry, I'm pinging you for little things but I really need some help

@MarkEWaite
Copy link
Contributor

sorry, I'm pinging you for little things but I really need some help

A change to the Jenkinsfile is one of those things that requires a ping of someone else unless you are a maintainer of the plugin. I'll start a build of the plugin with your modified Jenkinsfile so that you can see the results.

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Optional comment on the Jenkins version being tested

Jenkinsfile Outdated Show resolved Hide resolved
update Jenkins version to 2.387.2

Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
@VignanMudedla
Copy link
Contributor Author

Hey @MarkEWaite Thanks a lot for your support!!
Can you review this PR and Deeper spotbugs checks PR ?

Jenkinsfile Outdated Show resolved Hide resolved
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Limit the number of platforms to reduce CI cost and assure fast evaluation of pull requests

remove windows java 11 build config

Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Jenkinsfile Outdated Show resolved Hide resolved
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

I have confirmed that the changes to the Jenkinsfile run as expected in a recent job. I had missed two items in previous reviews. Included in this review.

pom.xml Show resolved Hide resolved
Jenkinsfile Outdated Show resolved Hide resolved
MarkEWaite added a commit to MarkEWaite/repository-permissions-updater that referenced this pull request Apr 15, 2023
I've been reviewing proposed changes from @VignanMudedla and see that
there is only one person in the
https://github.com/orgs/jenkinsci/teams/javadoc-plugin-developers
group (Kohsuke).  My adoption of the plugin will focus on merging pull
requests from VignanMudedla and from dependabot, including:

* jenkinsci/javadoc-plugin#96 - update base Jenkins version
* jenkinsci/javadoc-plugin#97 - more spotbugs checks
* jenkinsci/javadoc-plugin#94 - dependabot

I'll also close the following outdated dependabot pull request:

* jenkinsci/javadoc-plugin#93
added build config for windows java 17

Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
@MarkEWaite MarkEWaite changed the title updated parent pom and base jenkins version Require Java 11 and Jenkins 2.361.4 or newer Apr 15, 2023
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Still needs the bom-2.361.x change and the latest bom version in that change. When you resolve a comment, I assume it means you addressed the concern, but in this case, the concern was not addressed. Let's get more benefits from the Java 11 transition, not just update the minimum Jenkins version.

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Thanks!

@VignanMudedla
Copy link
Contributor Author

Still needs the bom-2.361.x change and the latest bom version in that change. When you resolve a comment, I assume it means you addressed the concern, but in this case, the concern was not addressed. Let's get more benefits from the Java 11 transition, not just update the minimum Jenkins version.

I just committed that change!!
Extremely sorry for marking it resolved before making the change
this is because I'm new to GitHub

@MarkEWaite
Copy link
Contributor

because I'm new to GitHub

Delighted that you're willing to learn and very happy to have your contribution. Thanks again. Once this change is merged I have two more changes that I will push through, then the plugin will release so that you can see the results of your efforts in a plugin release.

@MarkEWaite MarkEWaite merged commit bb4e4ce into jenkinsci:master Apr 15, 2023
@VignanMudedla VignanMudedla deleted the update-parent-pom-and-jenkins-version branch April 15, 2023 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants