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

actions/setup-java@v2 - Documentation #134

Merged
merged 10 commits into from Mar 15, 2021

Conversation

maxim-lobanov
Copy link
Contributor

@maxim-lobanov maxim-lobanov commented Mar 8, 2021

Description:
This pull-request updates documentation according to setup-java@v2

Looks like we had a ton of different use-cases in README file and it becomes unreadable. I have decided to leave multiple base use-cases in README and move other use-cases to the separate file advanced-usage.md.
Also, added guide for migration from V1 to V2 since it brings a bunch of breaking changes.

I would be very appreciate for some review and suggestions about wording to sound native :)

Note: we have decided to split changes to a few pull-requests to simplify review process:

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/switching-to-v2.md Outdated Show resolved Hide resolved
docs/switching-to-v2.md Outdated Show resolved Hide resolved
docs/switching-to-v2.md Outdated Show resolved Hide resolved
docs/switching-to-v2.md Outdated Show resolved Hide resolved
docs/advanced-usage.md Outdated Show resolved Hide resolved
docs/advanced-usage.md Outdated Show resolved Hide resolved
docs/advanced-usage.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: George Adams <george.adams@microsoft.com>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/advanced-usage.md Outdated Show resolved Hide resolved
docs/advanced-usage.md Outdated Show resolved Hide resolved
docs/advanced-usage.md Outdated Show resolved Hide resolved
docs/switching-to-v2.md Outdated Show resolved Hide resolved
Co-authored-by: Konrad Pabjan <konradpabjan@github.com>
README.md Outdated
- V2 requires you to specify distribution along with the version. V1 defaults to Zulu OpenJDK, only version input is required. Follow [the migration guide](docs/switching-to-v2.md) to switch from V1 to V2

## Usage
Input `distribution` is mandatory. See [Supported distributions](../README.md#Supported-distributions) section for the list of available options.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Input `distribution` is mandatory. See [Supported distributions](../README.md#Supported-distributions) section for the list of available options.
Input `distribution` is mandatory. See [Supported distributions](../README.md#Supported-distributions) for a list of available options.

@giltene
Copy link
Contributor

giltene commented Mar 10, 2021

The use of "adoptium" as a distribution name that is actuality will be getting the AdoptOpenJDK distro builds seems wrong here. So are the links to the AOJ site instead of the Adoptium site.

AFAIK Adoptium as an Eclipse project is a "...continuation of the original AdoptOpenJDK mission", but Eclipse Adoptium builds will be Eclipse Adoptium builds, and not AdoptOpenJDK builds. Adoptium and AdoptOpenJDK are two separate distros, produced by separate organizations, using separate tests and governance, and [potentially] separate terms of use. The actual binaries will [presumably] show different distro names in their version strings. And for a given java version sepcified [e.g. a past 8.0.252, or a future 11.0.19] there may be distro binaries available from one of those two distros, or both, or neither.

Explicitly specifying one distro in the "distribution" field and getting another distorts binaries would clearly be wrong.

So either the word "adoptium" should be changed to something that properly refers to AdoptOpenJDK distro and will not conflict with Adoptium distro when it starts providing binaries, or the meaning of using "adoptium" should change to [only] using bits provided by the Eclipse Adoptium project, and not bits that come from some other distro.

Note: If the choice is made to change the name used to specify the AdoptOpenJDK distro, that choice should likely avoid using the word "openjdk" in the distro name for the obvious reasons (including trademark rules and the avoidance of assymetric use of the term openjdk across the various choices for distros of OpenJDK). Names like "adopt" and "AOJ" probably work in this regard.

@konradpabjan
Copy link
Collaborator

konradpabjan commented Mar 10, 2021

Thanks for the info @giltene

My assumption based off of https://blog.adoptopenjdk.net/2020/06/adoptopenjdk-to-join-the-eclipse-foundation/ was that down the road AdoptOpenJDK would be transitioned to Eclipse Adoptium and things would be largely the same. Two separate distros seems weird (although it's possible of course). I think the confusion here is because the rename that was announced hasn't happened yet and it will probably take some time.

As part of a move to the Eclipse Foundation, we will undergo a project rename.

As well as

In time, some services, websites, and APIs would move to new URLs, and some package names might change. Everything will be announced well in advance on a per-service basis with long transition periods.

Even if you go to https://projects.eclipse.org/projects/adoptium/downloads there is currently nothing.

It is confusing though and I like the suggestion to use adopt. Both users of AdoptOpenJDK and Eclipse Adoptium or Adoptium should be able to recognize it. @maxim-lobanov lets go with that (lots of things to rename unfortunately...)

@maxim-lobanov maxim-lobanov merged commit 20a04c5 into implement-v2-installers Mar 15, 2021
@maxim-lobanov maxim-lobanov deleted the implement-v2-docs branch March 15, 2021 08:09
@giltene
Copy link
Contributor

giltene commented Mar 15, 2021

In the "basic" example description in the README, we should change the distro selection from "adopt" to "zulu". (We can also mention the other distro options explicitly in the comment, to avoid having people jump somewhere else just to find out possible spelling). For the 60K+ current users of V1, filling in zulu as the distribution (which is a new required property) will be the "compatible" entry, and is therefore the obvious example choice if only one choice is shown as an exmaple. It is the most straightforward way to not break (especially since there are lots of currently used versions in workflow that zulu provides and adopt does not).

@gdams
Copy link
Contributor

gdams commented Mar 16, 2021

@giltene no users will be broken? All V1 users will continue to be able to use Zulu going forwards. If they are explicitly making the decision to bump to V2 then they can also make the code change to choose their vendor. As pointed out before, AdoptOpenJDK is already preinstalled on all actions environments so using adopt in the example seems the most logical approach going forwards.

@konradpabjan
Copy link
Collaborator

@giltene We're going to update the basic example to have an example with Zulu as well https://github.com/actions/setup-java/pull/136/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R34-R44

Given that adopt is already pre-installed on our images, for consistency moving forward we'll be using adopt in examples so expect public documentation at some point to reflect those changes: https://docs.github.com/en/actions/guides/building-and-testing-java-with-maven

As mentioned if users are pinned to v1 then nothing will change or break. Only users pinned to @main will have workflows start to fail but we have a migration guide and we will have a change-log the day of the v2 release. We also actively don't recommend users pin to @main or @master (and instead to tags) as breaking changes can happen between major versions.

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.

None yet

6 participants