-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
website: restore pom.xml #15113
website: restore pom.xml #15113
Conversation
website/pom.xml
Outdated
<parent> | ||
<groupId>org.apache.druid</groupId> | ||
<artifactId>druid</artifactId> | ||
<version>27.0.0-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs updated to 28.0.0-SNAPSHOT
<node.version>v16.17.0</node.version> | ||
<npm.version>8.15.0</npm.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these the recommended docusaurus 2 versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They don't have a recommended NPM version and the recommended node version is 16.14 or later
Running |
<parent> | ||
<groupId>org.apache.druid</groupId> | ||
<artifactId>druid</artifactId> | ||
<version>28.0.0-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess this needs to be 29 snapshot now since #15121 has been merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this used for anything outside of building the website and interpolating the version variable used in the docs?? The variable interpolation got moved to druid-website-src
along with the rest of the stuff for a production build.
If not, we can probably delete it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it needs to match the rest of the pom versions if we are keeping this file at all i think, https://github.com/apache/druid/blob/master/pom.xml#L2128 has a profile that refers to this file. It seems useful to me to have a pom for the website that can run basic npm build and test commands the same as we have for web-console if its going to live in the same repository as the rest of druid stuff. whether or not it needs to build the full website or not i suppose is a different matter, but i think at least it should be able to run npm build and run the spellcheck (so that it can be done independently of whatever node/npm versions exist on the developers machine globally)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for example https://github.com/apache/druid/blob/master/distribution/asf-release-process-guide.md#set-version-and-make-a-tag still refers to bumping the website version so it matches everything else. it does look like maybe this part has been updated at least https://github.com/apache/druid/blob/master/distribution/asf-release-process-guide.md#update-druidstagedapacheorg, having not built the full website i'm not sure if it would ever make sense to try to drive that process through here instead of having to drive from the website-src repo...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks for taking the time to explain it! I'm pretty unfamiliar with Maven since it's not really in the typical docs workflow.
I haven't had time to figure out why the spellchecker doesn't run when I do something like mvn compile
though or if it runs, it doesn't return any feedback. Manually running it returns feedback on errors or success. The spellchecking script hasn't changed. The build runs and completes and we get feedback on it though
@317brian Is this PR still required or can we close this out? |
It's required but not tied to any particular release since we don't build the website with this repo anymore. The fix is a quality of life fix for contributors. |
we might want to consider removing the website profile from the 28 branch if we aren't going to merge and backport this |
If we need not build website with this repo, we could just remove the profile itself. The profile was added so we are bumping up the pom version in website module. That's no longer needed now. |
This restores the pom.xml file from right before the Docusaurus 2 upgrade. It's copied from the commit immediately prior to the upgrade from the 27.0 branch: https://github.com/apache/druid/blob/25e6e28e657767885f2126e1ec1e016c1d20e873/website/pom.xml