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

Refactor CI to run GraalVM native build in separate job #1055

Merged
merged 3 commits into from
Feb 11, 2024

Conversation

vorburger
Copy link
Member

matrix:
os: [ubuntu-latest, macos-latest]
runs-on: ${{ matrix.os }}
continue-on-error: ${{ matrix.experimental }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

experimental isn't part of the matrix for this job, I think it needs to be added, or else this could just hard-code continue-on-error: true for now if we want it to be true for both jobs

We should probably also remove continue-on-error: true once we start doing releases that depend on this, but I think it's OK to leave in for now while the graal stuff is being iterated on

@cushon
Copy link
Collaborator

cushon commented Feb 11, 2024

Thanks! This looks about like what I expected, but I don't see see any CI runs for this PR? https://github.com/google/google-java-format/actions/workflows/ci.yml

I'm not sure why it hasn't run, any ideas? I wonder if there's an error here preventing it from running that's not being reported.

Fix some typos--update references from `test` to `test-OpenJDK`, remove a stale reference to `matrix.experimental` from GraalVM tests
@cushon
Copy link
Collaborator

cushon commented Feb 11, 2024

Ah, neat, the built-in GitHub editor shows errors for the config file. I pushed a commit that I think fixes it.

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 0c8cc2b5..322f914e 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -81,14 +81,14 @@ jobs:
         shell: bash
         run: mvn test -B
 
-  test-OpenJDK:
+  test-GraalVM:
     name: "GraalVM on ${{ matrix.os }}"
     strategy:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest]
     runs-on: ${{ matrix.os }}
-    continue-on-error: ${{ matrix.experimental }}
+    continue-on-error: true
     steps:
       - name: Cancel previous
         uses: styfle/[email protected]
@@ -111,7 +111,7 @@ jobs:
 
   publish_snapshot:
     name: "Publish snapshot"
-    needs: test
+    needs: test-OpenJDK
     if: github.event_name == 'push' && github.repository == 'google/google-java-format' && github.ref == 'refs/heads/master'
     runs-on: ubuntu-latest
     steps:

@cushon
Copy link
Collaborator

cushon commented Feb 11, 2024

CI looks better now, I'll go ahead and merge but we can keep iterating if you disagree with any of the edits

@cushon cushon merged commit 1ae69f9 into google:master Feb 11, 2024
10 checks passed
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.

2 participants