Skip to content

Improve/fix GitHub Actions pipleline #101

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

Merged
merged 3 commits into from
Apr 10, 2025
Merged

Conversation

NamedPython
Copy link
Contributor

@NamedPython NamedPython commented Apr 9, 2025

Problems

  • Compatibility problem at the java setup
  • main.yml refers main branch, but default branch is master here.
  • There're no checks at the PR

Solutions

  • use setup-java@v4 and specify Java 8 and zulu distribution
  • fix main reference to master
  • split pipeline into gem-push and check. check will exec test at PullReq.

@NamedPython NamedPython self-assigned this Apr 9, 2025
@NamedPython NamedPython requested review from d-hrs and chikamura April 9, 2025 06:50
packages: write
contents: read
steps:
- uses: actions/checkout@v2
Copy link
Contributor

Choose a reason for hiding this comment

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

You might need to set up JDK 8 and ruby 2.7.

https://github.com/trocco-io/embulk-input-jira/blob/7ebeebcdb00f4a783334bbbbd6d27b91ab764aa6/.github/workflows/gem-push.yml

      - name: Set up Ruby 2.7
        uses: ruby/setup-ruby@v1
        with:
          ruby-version: 2.7
      - name: Set up JDK 1.8
        uses: actions/setup-java@v4
        with:
          java-version: 8
          distribution: "zulu"

Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, it would be ideal to prevent gem push unless all tests have passed.

@NamedPython NamedPython requested a review from chikamura April 9, 2025 11:57
Copy link
Contributor

@chikamura chikamura left a comment

Choose a reason for hiding this comment

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

LGTM

[IMO] The tests in check.yml and gem-push.yml are duplicated, so it may be better to merge them into a single file.

@NamedPython
Copy link
Contributor Author

@chikamura I think that's better to know the lint / test failings before merge. Thanks for your review, gonna merge.

@NamedPython NamedPython merged commit 8f8ca1e into master Apr 10, 2025
1 check passed
@NamedPython NamedPython deleted the modernize-the-gha-pipeline branch April 10, 2025 02:01
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