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

RAT-374: automatically update cli documentation #253

Merged

Conversation

Claudenw
Copy link
Contributor

@Claudenw Claudenw commented May 18, 2024

Fixes RAT-374
Fixes RAT-381 -- Inject maven verson info into java code.

Changes to update documentation:

  • Added pre-site exec-maven action in apache-rat to generate a file target/site/CLI_help.txt that contains the generated help text
  • Modified apache-rat index.apt.vm to include the CLI_help.txt during build process.
  • Modified apache-rat README-CLI.txt to point to online help for details, rather than including the text directly.
  • Added apache-rat-core VersionInfo to receive Maven injected version info and use it in output.

@Claudenw Claudenw requested a review from ottlinger May 18, 2024 13:44
@ottlinger ottlinger changed the title Rat-374 automatically update cli documentation RAT-374: automatically update cli documentation May 18, 2024

- name: Generate javadoc
run: ./mvnw -e -B -V -ntp javadoc:javadoc
run: ./mvnw -e -B -V -ntp javadoc:javadoc
Copy link
Contributor

Choose a reason for hiding this comment

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

In the past we had javadoc build failures that strangely did not break the site build and were only discovered in the release-preparation phase. Thus this build step is a "security net" to make sure that javadoc can be properly generated for the whole project!

@ottlinger
Copy link
Contributor

Let us have a look at this after #252 is in main.
I'm still not sure how to integrate the CLI output into the site build, but will have a look at it and also check for references in the webpage to the "old" *.txt files.

apache-rat/pom.xml Outdated Show resolved Hide resolved
@ottlinger
Copy link
Contributor

I did not find any meaningful references in the site project to

  • txt
  • CLI
  • README-
    in order to break any existing links when we consolidate the options list/help output in 0.17.

@ottlinger
Copy link
Contributor

@Claudenw should https://github.com/apache/creadur-rat/blob/master/apache-rat/README.txt#L13 be adapted as well or do we just remove the link here?

@Claudenw Claudenw force-pushed the RAT-374_automatically_update_CLI_documentation branch from 9848c47 to 83700ce Compare May 19, 2024 08:06
@Claudenw
Copy link
Contributor Author

@ottlinger after the merge of #252 this change becomes much smaller and clearer to read. Please review when you have a moment. I think that the Version.properties file is the correct way to go. As you can see it gets the version from the POM and just makes it available to the system.

@Claudenw Claudenw self-assigned this May 20, 2024
@Claudenw Claudenw marked this pull request as ready for review May 20, 2024 09:17
@Claudenw Claudenw requested a review from ottlinger May 20, 2024 09:17
@ottlinger
Copy link
Contributor

@Claudenw I still have a lot of pain with the double release just to have a version string. Would it make sense to define a property or write a value inside the app's manifest we set once we release, so that any SNAPSHOT is filtered out in the CLI?
I just checked that we write certain manifest attributes - we could add a special attribute when releasing and this filters out any SNAPSHOT while reading the property.

Otherwise we should rely on the property previousRatVersion that is defined in the main pom (this value is also used to generate the download page upon releasing)

WDYT? I am still in doubt whether this is worth the effort to just have a version string in the CLI output ....

@Claudenw
Copy link
Contributor Author

Claudenw commented May 20, 2024 via email

@ottlinger
Copy link
Contributor

@Claudenw
We follow the apache way:

  • Generate the release candidate via maven-release-plugin (after many manual tweaks to the changelog/dokcumentation/webpage)
  • Upload the artifacts to ASF nexus
  • Vote on the artifact
  • Publicly announce the release after making it available via Maven central and ASF download mirrors

As the release has to be voted on, we cannot change the artifact after the release :)

@Claudenw
Copy link
Contributor Author

Claudenw commented May 20, 2024 via email

@Claudenw
Copy link
Contributor Author

@ottlinger

After digging around a bit I found a solution that reads the MANIFEST if it is present by using the Package class. This implementation uses that to create the VersionInfo. VersionInfo simply makes sure the various values from the Package instance are not null by providing reasonable defaults.

Interestingly, VersionInfo can now be passed any class and will report the information for that class. So our UIs can log what version of the UI is running and what version of the core if we desire.

Copy link
Contributor

@ottlinger ottlinger left a comment

Choose a reason for hiding this comment

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

Sry for the noise - as we rebuild the JAR via maven-release-plugin the correct version is injected/generated. The site build has to happen from the release branch in order for the site to contain the correct information.

Pls add a changelog entry and possibly edit the other CLI-txt file.

@Claudenw
Copy link
Contributor Author

@ottlinger The correct version is injected by changing the POM during the build process correct? So the version would be correct during the build and the previous solution would have worked. However, this solution is much cleaner.

@Claudenw Claudenw merged commit 716c1a3 into apache:master May 21, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants