-
Notifications
You must be signed in to change notification settings - Fork 105
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
No snapshot releases for OSS #1355
Conversation
✅ Deploy Preview for hardcore-allen-f5257d ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
docs/antora.yml
Outdated
@@ -10,6 +10,7 @@ asciidoc: | |||
attributes: | |||
# The full major.minor.patch version, which is used as a variable in the docs for things like download links | |||
full-version: '6.0.0-SNAPSHOT' | |||
last-oss-version: '5.5.0' |
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.
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.
Agreed.
@TomaszGaweda whats the background on this? Do we have a JIRA?
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's a user complain on Hazelcast Community Slack. And the user is right - we point to a version that does not exist.
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.
can you pass the Slack link pls?
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.
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.
Should be on 5.4 and 5.5 I think. @oliverhowell has some magic labels to do it automatically ;)
@oliverhowell whats "has some magic labels to do it automatically "? :)
It's this - it allows this change to backported, but don't update versions in the documentation, which is where DI-321 comes in.
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.
thanks @JackPGreen
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.
Guys - I agree that some automation may be necessary. IMHO best way is to merge this PR and create a DevInfra ticket to update release pipeline. Now we are just confusing users without any good reasons for them.
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.
Guys - I agree that some automation may be necessary. IMHO best way is to merge this PR and create a DevInfra ticket to update release pipeline. Now we are just confusing users without any good reasons for them.
I reluctantly agree.
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 agree. Lets not block this. We have https://hazelcast.atlassian.net/browse/DI-321
But prefer to see response to #1355 (comment) and if other places need updating then either we do it here or with another follow-up PR. Otherwise this PR feels incomplete
Could you clarify the scope of this PR, please?
The Slack discussion centres around the docs referring to
I don't think these are directly related to that. I've had a look and I can see there are (lots) of other outstanding places that need changing:
I'm not suggesting we do everything in this PR, but it'd be good to be clear about what were fixing here, and record the rest for later? |
docs/antora.yml
Outdated
@@ -10,6 +10,7 @@ asciidoc: | |||
attributes: | |||
# The full major.minor.patch version, which is used as a variable in the docs for things like download links | |||
full-version: '6.0.0-SNAPSHOT' | |||
last-oss-version: '5.5.0' |
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.
Just thinking again, is this the right name?
I think it should be os-version
and ee-version
to force you to think about what version you want to reference.
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.
Looks like this is only for OSS as we always release EE (so not needed)
May be latest-oss-version
. This will also cover the case where we are in the process of releasing current MAJOR/MINOR
I think prefer its prepended with some test to make it very clear
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.
Looks like this is only for OSS as we always release EE (so not needed)
Today, yes - but 3 months ago it was different.
If you aren't aware of the nuance it's easy to use full-version
not realising it's really EE-only in some cases.
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 agree with Jack's naming suggestion - I was looking at this PR last week and there's something about the overall logic that was not quite right. If the versions are diverging we need a variable for each that we can use in the various places.
We also need to clarify which versions are released in the release notes. The user started from the release notes and naturally assumed that the OS version was available as AFAICT we've never added this detail to the RN since the os/ee divergence. |
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.
Thanks for raising this, Tomasz - bit of a can of worms but I think these changes would enable us to fix the broken link issues and schedule further changes to docs that use these variables.
We'll need to think about backport strategy for these changes as we'll need variable backported but value will differ.
Co-authored-by: Oliver Howell <[email protected]>
Co-authored-by: Oliver Howell <[email protected]>
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.
Thanks @TomaszGaweda I suggested changes to make the rest of the variables match the new name
Co-authored-by: Oliver Howell <[email protected]>
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.
Thanks @TomaszGaweda
FYI @oliverhowell @JackPGreen @fawazghali - I've created https://hazelcast.atlassian.net/browse/DEX-304 to address the rest of the places where we need to reflect on our new approach. Thanks for reviews! |
Following on from hazelcast#1355 (comment), ensure the versions of referenced Docker images are correctly differentiating between OS & EE. [Slack discussion](https://hazelcast.slack.com/archives/C035HQET5/p1733424118024969) _Partially_ addresses: - [DOC-238](https://hazelcast.atlassian.net/browse/DOC-238) - [DEX-304](https://hazelcast.atlassian.net/browse/DEX-304)
Following on from #1355 (comment), ensure the versions of referenced Docker images are correctly differentiating between OS & EE. Note - not backporting to `v/5.5` as [doesn't contain these properties](https://github.com/hazelcast/hz-docs/blob/v/5.5/docs/antora.yml). [Slack discussion](https://hazelcast.slack.com/archives/C035HQET5/p1733424118024969) _Partially_ addresses: - [DOC-238](https://hazelcast.atlassian.net/browse/DOC-238) - [DEX-304](https://hazelcast.atlassian.net/browse/DEX-304)
…253] (#1382) This PR adds an action that will, when a PR is opened, check the resolvability of any external links. External links are defined as those using the `http` scheme - internal links _between_ documentation is already covered by our existing dead links check. Action is deliberately modular to be be easily applied to other docs repos. This runs whenever a PR is raised, which is sub-optimal because: - links can break at any time as they reference external content - there's no guarantees the breakage is anything to do with the new PR But the other alternative - a scheduled check - would have difficulty in knowing _who_ to target a failure communication at to resolve the issues (other projects notify Slack channels, with varying degrees of responses). An example of the output produced by this action can be found [here](https://github.com/hazelcast/hz-docs/actions/runs/11901935786). Note that the external link check *fails* because of dead links, and won't pass until the following are addressed: - #1355 - #1389 - `URL 'https://raw.github.com/olivernn/lunr.js/master/lunr.min.js' had status 404 (found in node_modules/lunr/index.html)` We should consider whether adding a check that we know will fail is a good idea - comparing the annoyance of a failing (non-blocking) test against the coverage that at least we can ensure things don't get _worse_. Fixes: [DOC-253](https://hazelcast.atlassian.net/browse/DOC-253) [DOC-253]: https://hazelcast.atlassian.net/browse/DOC-253?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
The test developed in hazelcast#1382 flagged multiple dead links. This PR fixes those _seperately_ to prove the status of the test. Note the changes from `install-hazelcast.adoc` ~~copy from~~ overlap with hazelcast#1355.
Community Slack thread: https://hazelcastcommunity.slack.com/archives/C015Q2TUBKL/p1730372380120239?thread_ts=1730371999.914789&cid=C015Q2TUBKL