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

No snapshot releases for OSS #1355

Merged
merged 5 commits into from
Nov 20, 2024
Merged

Conversation

TomaszGaweda
Copy link
Contributor

@TomaszGaweda TomaszGaweda commented Oct 31, 2024

  • Introduced property to set last OSS version (it can be different than EE version)
  • removed all snapshot version-related parts from OSS install guide

Community Slack thread: https://hazelcastcommunity.slack.com/archives/C015Q2TUBKL/p1730372380120239?thread_ts=1730371999.914789&cid=C015Q2TUBKL

Copy link

netlify bot commented Oct 31, 2024

Deploy Preview for hardcore-allen-f5257d ready!

Name Link
🔨 Latest commit c1db71f
🔍 Latest deploy log https://app.netlify.com/sites/hardcore-allen-f5257d/deploys/673db99f9edd3c0008b3152f
😎 Deploy Preview https://deploy-preview-1355--hardcore-allen-f5257d.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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'
Copy link
Contributor

@JackPGreen JackPGreen Oct 31, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks @JackPGreen

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

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

@JackPGreen
Copy link
Contributor

JackPGreen commented Nov 8, 2024

Could you clarify the scope of this PR, please?

Community Slack thread: https://hazelcastcommunity.slack.com/archives/C015Q2TUBKL/p1730372380120239?thread_ts=1730371999.914789&cid=C015Q2TUBKL

The Slack discussion centres around the docs referring to 5.5.2, but the last public release being 5.5.0

  • Introduced property to set last OSS version (it can be different than EE version)
  • removed all snapshot version-related parts from OSS install guide

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?

@JackPGreen JackPGreen self-requested a review November 8, 2024 17:05
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'
Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor

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.

@oliverhowell
Copy link
Contributor

Could you clarify the scope of this PR, please?

Community Slack thread: https://hazelcastcommunity.slack.com/archives/C015Q2TUBKL/p1730372380120239?thread_ts=1730371999.914789&cid=C015Q2TUBKL

The Slack discussion centres around the docs referring to 5.5.2, but the last public release being 5.5.0

  • Introduced property to set last OSS version (it can be different than EE version)
  • removed all snapshot version-related parts from OSS install guide

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?

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.

Copy link
Contributor

@oliverhowell oliverhowell left a 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.

Copy link
Contributor

@oliverhowell oliverhowell left a 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

Copy link
Contributor

@oliverhowell oliverhowell left a comment

Choose a reason for hiding this comment

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

Thanks @TomaszGaweda

@TomaszGaweda
Copy link
Contributor Author

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!

@TomaszGaweda TomaszGaweda merged commit 9bcca34 into hazelcast:main Nov 20, 2024
7 checks passed
JackPGreen added a commit that referenced this pull request Nov 25, 2024
The test developed in #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 #1355.
JackPGreen added a commit to JackPGreen/hz-docs that referenced this pull request Dec 5, 2024
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)
JackPGreen added a commit that referenced this pull request Dec 6, 2024
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)
JackPGreen added a commit that referenced this pull request Dec 9, 2024
…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
JackPGreen added a commit to JackPGreen/hz-docs that referenced this pull request Dec 19, 2024
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.
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.

4 participants