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.

Sign up for GitHub

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

Jump to bottom

First attempt at GH Action updates #310

Merged
merged 2 commits into from
Nov 9, 2023
Merged

First attempt at GH Action updates #310

merged 2 commits into from
Nov 9, 2023

Conversation

karianna
Copy link
Member

@karianna karianna commented Nov 2, 2023

No description provided.

d3r3kk
d3r3kk requested changes Nov 2, 2023
View reviewed changes
Copy link
Collaborator

@d3r3kk d3r3kk left a comment

Choose a reason for hiding this comment

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

Looks mostly good, but a few recommendations. Keeping to major versions of actions is best practice, AFAIK, and swapping for the Microsoft Build of OpenJDK seems relevant 😁.

.github/workflows/codeql-analysis.yml Outdated
@@ -38,11 +38,11 @@ jobs:

steps:
- name: Checkout repository
uses: actions/checkout@v2
uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Latest suggested version is just v4. Should do... and will keep updating until v4 is no longer updated (and v5 becomes the main version to follow).

Suggested change
uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
uses: actions/checkout@v4

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can a link to an issue be provided? I'm trying to understand what is happening here in light of our upcoming changes to integrate the MS signing policy and our integration with jreleaser

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just a GitHub Action version update for CI, it's separate to any jreleaser efforts.

.github/workflows/early_access.yml Outdated
@@ -12,13 +12,13 @@ jobs:
if: startsWith(github.event.head_commit.message, '[maven-release-plugin]') != true

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
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
- uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
- uses: actions/checkout@v4

.github/workflows/early_access.yml

- name: Set up JDK 11
uses: actions/setup-java@v2
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

V2 is good. See this article...

Suggested change
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
uses: actions/setup-java@v2

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah but v3.13.0 is the latest :-)

.github/workflows/early_access.yml Outdated
with:
java-version: 11
distribution: 'adopt'
distribution: 'temurin'
Copy link
Collaborator

Choose a reason for hiding this comment

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

You meant Microsoft, right? 😁

Suggested change
distribution: 'temurin'
distribution: 'microsoft'

.github/workflows/maven.yml Outdated
@@ -21,13 +21,13 @@ jobs:
java: [11, 17]

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
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
- uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
- uses: actions/checkout@v4

.github/workflows/maven.yml Outdated
with:
java-version: ${{ matrix.java }}
distribution: 'adopt'
distribution: 'temurin'
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
distribution: 'temurin'
distribution: 'microsoft'

.github/workflows/publish.yml Outdated
@@ -10,23 +10,23 @@ jobs:

steps:
- name: Checkout project
uses: actions/checkout@v2
uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
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
uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
uses: actions/checkout@v4

.github/workflows/publish.yml
with:
ref: main
fetch-depth: 0

- name: Cache Maven
uses: actions/cache@v2.1.4
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84 # v3.3.2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the latest major version to use for cache is v3.

Suggested change
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84 # v3.3.2
uses: actions/cache@v3

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, it's 3.3.2, it's safer to use teh commit hash and a comment, dependabot knows what to do going forward

.github/workflows/publish.yml
with:
path: ~/.m2/repository
key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
restore-keys: ${{ runner.os }}-m2

- name: Setup Java JDK
uses: actions/setup-java@v2
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
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
uses: actions/setup-java@0ab4596768b603586c0de567f2430c30f5b0d2b0 # v3.13.0
uses: actions/setup-java@v2

.github/workflows/publish.yml Outdated
with:
java-version: '11'
distribution: 'adopt'
distribution: 'temurin'
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
distribution: 'temurin'
distribution: 'microsoft'

@karianna karianna requested review from d3r3kk and kcpeppe November 6, 2023 20:59
@karianna
Copy link
Member Author

karianna commented Nov 6, 2023

@d3r3kk FYI - the Better security practice is now to reference the hash commits of GH actions (as vX is an alias that can be updated under the hood) and comment on the side, Dependabot has been updated to cope with this new practice.

@d3r3kk
Copy link
Collaborator

d3r3kk commented Nov 7, 2023

@d3r3kk FYI - the Better security practice is now to reference the hash commits of GH actions (as vX is an alias that can be updated under the hood) and comment on the side, Dependabot has been updated to cope with this new practice.

Oh interesting. Then I rescind all comments to that effect.

d3r3kk
d3r3kk approved these changes Nov 9, 2023
View reviewed changes
Copy link
Collaborator

@d3r3kk d3r3kk left a comment

Choose a reason for hiding this comment

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

With the updated info on best practices, I say 🚢 it.

@d3r3kk d3r3kk merged commit 4342691 into microsoft:main Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d3r3kk d3r3kk d3r3kk approved these changes

@kcpeppe kcpeppe Awaiting requested review from kcpeppe

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@karianna @d3r3kk @kcpeppe