Skip to content

Conversation

@ascheman
Copy link
Contributor

Besides a fix for #1288 the change also contains

  • a respective CI build (mvn ... site) to detect such problems early in the future
  • a simplification for GH CI triggers (build on each push, not only master and PRs)

The latter leads to CI builds for each development change (which is a major purpose of an automatic build and helps in particular for forks/new contributors who cannot issue a automatic build by creating a PR). Having said that, the respective commit could be dropped if there is a good reason not to follow this practice.


- name: 'Run default (non-native) build'
run: ./mvnw verify -Dmrm=false -V -B -ntp -e -s .mvn/release-settings.xml
run: ./mvnw verify site -Dmrm=false -V -B -ntp -e -s .mvn/release-settings.xml
Copy link
Member

Choose a reason for hiding this comment

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

I'd not interleave site into this one... @gnodet ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, maybe another job would be better specifically for the site.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it to another Job

@ascheman ascheman force-pushed the bugfix/1288-fix-skin branch 2 times, most recently from 044be95 to 3ea19ee Compare March 19, 2025 06:27
as the purpose of CI is to reveal errors early (in particular on forks).

This contributes to fix apache#1288 but might be helpful for other changes as well.
Otherwise this commit should be stripped from the build before merging
by switching to the latest Maven Fluido Skin which is compatible with
the currently used M-Site-P (3.21.0).
@ascheman ascheman force-pushed the bugfix/1288-fix-skin branch from 10f45af to e435124 Compare March 19, 2025 06:48
Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

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

LGTM

@ascheman
Copy link
Contributor Author

ascheman commented Apr 8, 2025

Would anyone mind to merge this, @gnodet or @cstamas ?

@cstamas cstamas merged commit cb2c732 into apache:master Apr 8, 2025
6 checks passed
@github-actions github-actions bot added this to the 2.0.0-rc-4 milestone Apr 8, 2025
@ascheman ascheman deleted the bugfix/1288-fix-skin branch April 8, 2025 08:54
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.

3 participants