Skip to content
This repository was archived by the owner on Feb 9, 2021. It is now read-only.

Update README#27

Merged
bryanmacfarlane merged 1 commit into
actions:masterfrom
eregon:patch-1
Jan 17, 2020
Merged

Update README#27
bryanmacfarlane merged 1 commit into
actions:masterfrom
eregon:patch-1

Conversation

@eregon
Copy link
Copy Markdown
Contributor

@eregon eregon commented Sep 21, 2019

  • Use more realistic versions.
  • architecture is not used for this action.
  • Remove redundant name: in example.

* Use more realistic versions.
* `architecture` is not used for this action.
* Remove redundant `name:` in example.
@eregon
Copy link
Copy Markdown
Contributor Author

eregon commented Nov 24, 2019

@damccorm Could you review and merge this PR?

@damccorm
Copy link
Copy Markdown
Contributor

@bryanmacfarlane could you take a look?

@ioquatix
Copy link
Copy Markdown

ioquatix commented Dec 29, 2019

@damccorm @bryanmacfarlane both myself and @eregon are ruby core team members. Can you please add both of us as maintainers of this repo/action? Or can we at least discuss if that's possible or not? Thanks.

@ioquatix
Copy link
Copy Markdown

Could be great to add @headius as he is the maintainer of JRuby too, so we get everyone with a finger in the pie so to speak.

@eregon
Copy link
Copy Markdown
Contributor Author

eregon commented Dec 30, 2019

Indeed, it looks to me this action already has no active maintainers recently, as illustrated by
https://github.com/actions/setup-ruby/graphs/contributors

So it would be great if the current maintainers would be more active or if the Ruby community could help maintain it. Otherwise I think it will eventually result in a hard fork, which seems obviously suboptimal.

@bryanmacfarlane
Copy link
Copy Markdown
Member

@ioquatix - I think adding ruby maintainers as contributors here is a good idea. We may need some communication around releases so our on call engineers are informed but I can work with you on that.

@ioquatix
Copy link
Copy Markdown

@bryanmacfarlane sounds good to me!

@eregon
Copy link
Copy Markdown
Contributor Author

eregon commented Jan 4, 2020

@bryanmacfarlane Could you review this PR and merge it?

I think it's obvious fixes here and it's safe to merge as-is.

Comment thread README.md
strategy:
matrix:
ruby: [ '2.x', '3.x' ]
ruby: [ '2.5.x', '2.6.x' ]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just to make sure (since I'm not a ruby expert), 3.x doesn't exist, right? If so, then this makes alot of sense.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Exactly, there is no Ruby 3 version yet: https://en.wikipedia.org/wiki/Ruby_(programming_language)#Table_of_versions
And every Ruby software will want to bind to a given minor number, so 2.5.x and 2.6.x is a good example.

Comment thread README.md
- uses: actions/setup-ruby@v1
with:
ruby-version: ${{ matrix.ruby }}
architecture: 'x64'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

right now, I think we need the x64 since we the current setup actions position is VM cache only and I believe it only has 64 bit

Copy link
Copy Markdown
Contributor Author

@eregon eregon Jan 7, 2020

Choose a reason for hiding this comment

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

I looked at this action code and nowhere does it reference architecture.
https://github.com/actions/setup-ruby/blob/master/action.yml does not mention it either.
So my conclusion is the argument seems unused.

I also would assume in 99% cases people would want the default architecture, not e.g. 32-bit on a 64-bit Ubuntu worker as that just wouldn't run well.

@eregon
Copy link
Copy Markdown
Contributor Author

eregon commented Jan 17, 2020

@bryanmacfarlane What's preventing to merge this?

It's just fixing the README. I don't understand how it can take so long to merge. There is no risk.

@bryanmacfarlane bryanmacfarlane merged commit 18c61ab into actions:master Jan 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants