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

Add GitHub Actions CI #406

Closed
wants to merge 2 commits into from
Closed

Add GitHub Actions CI #406

wants to merge 2 commits into from

Conversation

MSP-Greg
Copy link
Contributor

@MSP-Greg MSP-Greg commented Mar 18, 2020

Two commits:

  1. 'Rakefile - update for Windows mingw & mswin' - minor Windows specific changes to allow compiling on mingw & mswin

  2. 'Add GitHub Actions - test Ubuntu, macOS, Windows' - two workflow files

    1. Pass - Test MRI Rubies, 2.2 thru head, on Ubuntu, macOS, & Windows. Test TruffleRuby on Ubuntu & macOS.

    2. Fail - Test JRuby 9.2 and JRuby head on Ubuntu & macOS. All jobs are 'allow-failure'.

@MSP-Greg
Copy link
Contributor Author

MSP-Greg commented Mar 18, 2020

The main reason I submitted this PR was to achieve parity with the MRI platforms used in the main ruby/ruby CI. Having CI pass here and then cause CI failures in ruby/ruby is a PITA. The Ruby master builds used in Actions CI are fully tested, unlike the builds on other CI platforms.

For the impatient, Actions CI is typically faster than Travis, even with more jobs...

EDIT: When adding Actions workflow files via a PR to a repo without Actions CI, they often don't run. See https://github.com/MSP-Greg/json/actions/runs/57879745 for results.

One file, two jobs

1. pass - Test MRI Rubies, 2.2 thru head, on Ubuntu, macOS, & Windows.  Test TruffleRuby on Ubuntu & macOS.

2. fail - Test JRuby 9.2 and JRuby head on Ubuntu & macOS.  All jobs are 'allow-failure'.
@MSP-Greg
Copy link
Contributor Author

MSP-Greg commented Apr 1, 2020

Rebased and updated to make use of changes to Actions matrix api

See:
https://github.com/MSP-Greg/json/actions/runs/67872330

strategy:
fail-fast: false
matrix:
os: [ ubuntu, macos, windows ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not mix Linux/macOS and Windows platforms in the one workflow.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am curious, why do you prefer separate workflows?
I usually prefer like this if there isn't much OS-specific logic, as it avoids duplication and makes it significantly easier to maintain (1 place to change vs 3).

Copy link
Collaborator

Choose a reason for hiding this comment

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

The maintainability is low about mixing *nix and Windows for my experience.

Copy link
Contributor

Choose a reason for hiding this comment

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

That may have been true in the past at the beginning of GitHub Actions, but I don't think it's the case anymore, especially with improvements in ruby/setup-ruby. In fact many gems already use such an approach and in my experience, it's successful.

From a maintainability point of view it certainly looks better to me to have the steps repeated the minimum number of times (2 times here for allowed failures, instead of 3-4 if we have one job definition per OS).

steps:
- uses: actions/checkout@v2
- name: Set up Ruby
uses: MSP-Greg/setup-ruby-pkgs@v1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you remove the your actions? The maintainers can't control it.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's needed to install ragel in a platform-agnostic way.
MSP-Greg/setup-ruby-pkgs@v1 just uses ruby/setup-ruby for the setting up Ruby part.

@MSP-Greg
Copy link
Contributor Author

@hsbt

Can you remove the your actions? The maintainers can't control it.

I'm happy to do one or both of the following:

  1. Add additional maintainers
  2. Move it to another account

@hsbt
Copy link
Collaborator

hsbt commented Apr 11, 2020

I'm happy to do one or both of the following:

I have no motivation about that.

@hsbt
Copy link
Collaborator

hsbt commented Apr 11, 2020

Unfortunately, Greg did reject my suggestion.

@hsbt hsbt closed this Apr 11, 2020
@eregon
Copy link
Contributor

eregon commented Apr 12, 2020

@hsbt I don't understand why you closed this PR, could you elaborate?

What's the issue with setup-ruby-pkgs? As far as I can see here @MSP-Greg offered to add additional maintainers or move it to another account, but you didn't say if you wanted any of that or something else.

Also I don't see Greg rejecting any suggestion here.

@hsbt
Copy link
Collaborator

hsbt commented Apr 12, 2020

He refused the maintainer's requests on this and ruby/psych#444. I never collaborate with these attitude.

@MSP-Greg MSP-Greg deleted the add-actions branch January 15, 2022 14:38
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

3 participants