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

Trigger the release dry run right after each jdk build is done #923

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

sophia-guo
Copy link
Contributor

@sophia-guo sophia-guo commented Feb 12, 2024

Trigger the release dry run right after each jdk build is done
Provide the release publish build link per os, arch with pre-filled parameters

Close #911

Copy link

Thank you for creating a pull request!

Please check out the information below if you have not made a pull request here before (or if you need a reminder how things work).

Code Quality and Contributing Guidelines

If you have not done so already, please familiarise yourself with our Contributing Guidelines and Code Of Conduct, even if you have contributed before.

Tests

Github actions will run a set of jobs against your PR that will lint and unit test your changes. Keep an eye out for the results from these on the latest commit you submitted. For more information, please see our testing documentation.

In order to run the advanced pipeline tests (executing a set of mock pipelines), it requires an admin to post run tests on this PR.
If you are not an admin, please ask for one's attention in #infrastructure on Slack or ping one here.
To run full set of tests, use "run tests"; a subset of tests on specific jdk version, use "run tests quick 11,21"

@sophia-guo sophia-guo marked this pull request as draft February 12, 2024 17:37
@sophia-guo sophia-guo force-pushed the release1 branch 5 times, most recently from 81fca4f to 77c1795 Compare February 12, 2024 18:53
Signed-off-by: Sophia Guo <sophia.gwf@gmail.com>
@sophia-guo
Copy link
Contributor Author

The linter warning is the methods is 31, the default maxMethods=30 https://codenarc.org/codenarc-rules-size.html#methodcount-rule

@sophia-guo sophia-guo marked this pull request as ready for review February 12, 2024 22:33
Copy link
Contributor

@smlambert smlambert left a comment

Choose a reason for hiding this comment

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

LGTM, though noting a minor typo in comment

Eventually I could see dryrunReleasePublish(IndividualBuildConfig config) becoming publishRelease(IndividualBuildConfig config, BooleanParameterValue dryRun)

pipelines/build/common/build_base_file.groovy Outdated Show resolved Hide resolved
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

As I understand it this will happen as part of the top level openjdkXX-pipeline which sounds good to me!

Co-authored-by: Shelley Lambert <slambert@gmail.com>
@sophia-guo
Copy link
Contributor Author

As I understand it this will happen as part of the top level openjdkXX-pipeline which sounds good to me!

Correct. Start the dry run from the top level openjdkXX-pipeline

@sophia-guo sophia-guo merged commit a3e6f49 into adoptium:master Feb 13, 2024
6 of 7 checks passed
@sxa
Copy link
Member

sxa commented Feb 13, 2024

I didn't spot this at first but it looks like this failed the github linter check - possibly as a result of it going over the linter's limit for the number of methods in a class with new ones being added here:

  34    warning  Class Builder has 31 methods  MethodCount             

I can't see anything else that was flagged in there, but this will presumably cause subsequent PRs to the pipelines to fail the checks too.

@sophia-guo
Copy link
Contributor Author

#923 (comment) @sxa

@sophia-guo
Copy link
Contributor Author

sophia-guo commented Feb 13, 2024

Eventually I could see dryrunReleasePublish(IndividualBuildConfig config) becoming publishRelease(IndividualBuildConfig config, BooleanParameterValue dryRun)

We can do that when the betaTrigger is finalized.

I see that has been done #914

issue opened #926

@sxa
Copy link
Member

sxa commented Feb 13, 2024

The linter warning is the methods is 31, the default maxMethods=30 https://codenarc.org/codenarc-rules-size.html#methodcount-rule

Yeah but we need to decide what to do with it since at the moment it's going to cause check failures which we don't want - either we need to reduce the number of methods or adjust the linter configuration to avoid problems for contributors. I'll raise an issue.

@sophia-guo
Copy link
Contributor Author

sophia-guo commented Feb 13, 2024

#926 can fix this temporarily, which downsize the method number to 30. But eventually either adjust the linter configuration or redefining the methods of builder is needed. Would be better to open a separate issue @sxa

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.

Auto build the release tool with the dry-run option in post build stage
4 participants