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

rework ITs to better integrate into skin documentation #33

Merged
merged 3 commits into from
Jun 10, 2022
Merged

Conversation

hboutemy
Copy link
Member

@hboutemy hboutemy commented Jun 6, 2022

see improvement on https://maven.apache.org/skins-archives/maven-fluido-skin-LATEST/sidebar/index.html
vs https://maven.apache.org/skins/maven-fluido-skin/sidebar/index.html

failing GH due to running "mvn site" instead of required "mvn package site"
I'll try to change that...

@hboutemy
Copy link
Member Author

hboutemy commented Jun 6, 2022

@slawekjaranowski IIRC, you're a GH expert, if you can please configure this branch to run "mvn package site" instead of "mvn site", that would fix the build...

Comment on lines 27 to 31
<skin>
<groupId>org.apache.maven.skins</groupId>
<artifactId>maven-fluido-skin</artifactId>
<!-- TODO check with next parent, and next m-site-p -->
<!-- <version>${project.version}</version> -->
<version>1.10.0</version>
<version>${project.version}</version><!-- keep using current version, even if it implies running mvn -Prun-its,reporting install site -->
</skin>
Copy link
Member

Choose a reason for hiding this comment

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

We have IT test which use current build skins.

IMHO project should used latest release version - even more we have the same definition in parent and we needn't here it.

all project should be build by the same standard command

Copy link
Member Author

@hboutemy hboutemy Jun 7, 2022

Choose a reason for hiding this comment

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

  1. having the skin release using an older skin release is not easy to understand
  2. if we do this, we absolutely need to remove the sample page, because it does not test the current skin release https://maven.apache.org/skins/maven-fluido-skin/sample.html (if I had seen the previous change, I would have -1 it because it creates confusion)
  3. project is built with the same command as any project = what I fully agree with you is important: it's just the site that has a little unusual requirement for good reasons, given we're releasing a skin, then using it makes sense

@apache apache deleted a comment from slawekjaranowski Jun 8, 2022
@slawekjaranowski
Copy link
Member

@hboutemy build fixed

@hboutemy
Copy link
Member Author

thank you @slawekjaranowski

@hboutemy hboutemy merged commit 171292d into master Jun 10, 2022
@hboutemy hboutemy deleted the its branch June 10, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants