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.

Sign up for GitHub

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

Jump to bottom

Multiple CI updates #894

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Multiple CI updates #894

merged 1 commit into from
Nov 29, 2023

Conversation

shanedell
Copy link
Contributor

@shanedell shanedell commented Nov 7, 2023 •
edited
Loading

Multiple CI updates:

  • Add matrix to test different Java versions to main CI and nightly CI.
  • Cleaned up the CI workflow file a bit.
    • The steps were too compacted, believe new layout makes it easier to read and follow.
  • Add java_version to job names.
  • Remove sbt compile from windows jobs, seemed to cause windows issues with jaxb.
  • Believe the nightly CI maybe fixed with this update.
  • Possibly fixes Nightly tests failing #855 but will wait to see if a nightly build passes before closing.

Closes #772

@shanedell shanedell added the CI Issues related to CI workflows label Nov 7, 2023
@shanedell shanedell added this to the 1.4.0 milestone Nov 7, 2023
@shanedell shanedell self-assigned this Nov 7, 2023
mbeckerle
mbeckerle approved these changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@mbeckerle mbeckerle left a comment

Choose a reason for hiding this comment

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

Looks like your matrix is only Java 8.

Daffodil library tests on 8, 11, 17
(Not yet Java 21, though that is now released)

BTW: 17 is noticeably faster.

@shanedell
Copy link
Contributor Author

Looks like your matrix is only Java 8.

Daffodil library tests on 8, 11, 17
(Not yet Java 21, though that is now released)

BTW: 17 is noticeably faster.

Yeah I think you might be looking at a different job. The ratCheck and lint only uses Java 8 I think but if you look at the package and test step it has Java 8, 11 and 17.

@shanedell shanedell mentioned this pull request Nov 9, 2023
Closed
@scholarsmate
Copy link
Contributor

The matrix'ed build looks good to me in general. The matrix for the code format and rat checks is a bit overkill since we're only running one configuration for those (Java 8) jobs. Do you foresee adding more OSes, Node versions, or Java versions to these jobs? I don't.

I'll re-review when it passes CI.

@shanedell
Copy link
Contributor Author

@scholarsmate I mostly did this as it is the way done in Daffodil. I also thought it made a bit easier to know where you can change the versions at. Currently the CI will not pass till #895 is completed.

scholarsmate
scholarsmate approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@scholarsmate scholarsmate left a comment

Choose a reason for hiding this comment

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

+1, LGTM.

@shanedell shanedell force-pushed the jvm-matrix-ci-cleanup branch 2 times, most recently from b0ec913 to 9f5a194 Compare November 29, 2023 18:43
@shanedell
Multiple CI updates:
581ef06 
- Add matrix to test different Java versions to main CI and nightly CI.
- Cleaned up the CI workflow file a bit.
  - The steps were too compacted, believe new layout makes it easier to read and follow.
- Add java_version to job names.
- Remove sbt compile from windows jobs, seemed to cause windows issues with jaxb.
- Believe the nightly CI maybe fixed with this update.
- Possibly fixes apache#855 but will wait to see if a nightly build passes before closing.

Closes apache#772
@shanedell shanedell force-pushed the jvm-matrix-ci-cleanup branch from 9f5a194 to 581ef06 Compare November 29, 2023 19:04
@shanedell shanedell merged commit 095129c into apache:main Nov 29, 2023
21 checks passed
@shanedell shanedell deleted the jvm-matrix-ci-cleanup branch November 29, 2023 19:15
@shanedell shanedell mentioned this pull request Nov 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbeckerle mbeckerle mbeckerle approved these changes

@arosien arosien Awaiting requested review from arosien

@stevedlawrence stevedlawrence Awaiting requested review from stevedlawrence

@stricklandrbls stricklandrbls Awaiting requested review from stricklandrbls

@rthomas320 rthomas320 Awaiting requested review from rthomas320

@michael-hoke michael-hoke Awaiting requested review from michael-hoke

@NolanMatt NolanMatt Awaiting requested review from NolanMatt

@nlewis05 nlewis05 Awaiting requested review from nlewis05

@scholarsmate scholarsmate Awaiting requested review from scholarsmate

@lrbarber lrbarber Awaiting requested review from lrbarber

Assignees

@shanedell shanedell

Labels
CI Issues related to CI workflows
Projects
No open projects
daffodil-vscode v1.4.0
Status: Done
Milestone
1.4.0
Development

Successfully merging this pull request may close these issues.

Nightly tests failing Add CI build matrix for JVM versions
3 participants
@shanedell @scholarsmate @mbeckerle