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

Improve CI #44

Closed
wants to merge 5 commits into from
Closed

Conversation

w-masahiro-ct
Copy link
Contributor

@w-masahiro-ct w-masahiro-ct commented Dec 27, 2024

I improved CI.

@gjtorikian
Copy link
Owner

Ah, unfortunately, I can't accept the changes to setup-ruby. I like to ensure all my projects use the same actions I created so I don't have to think about it. Besides which, setup-languages already performs these operations: https://github.com/yettoapp/actions/blob/abbd75422c1118794948f6cc682efe5a7cf988b0/setup-languages/action.yml#L33-L40

Also, why delete the .ruby-version?

@w-masahiro-ct
Copy link
Contributor Author

I like to ensure all my projects use the same actions I created so I don't have to think about it.

I changed to use yettoapp/actions/setup-languages.

Also, why delete the .ruby-version?

Repositories of commonly used Gems such as Puma, Faraday, and RuboCop do not contain .ruby-version file.
I think Ruby version should not be specified in the Gem repository, so that is why I added matrix to CI.
However, it's not a big problem, so it's okay to restore the .ruby-version file👌

@w-masahiro-ct
Copy link
Contributor Author

w-masahiro-ct commented Dec 29, 2024

For reference, below is the .gitignore for Ruby from github/gitignore.
https://github.com/github/gitignore/blob/main/Ruby.gitignore#L46-L50

@gjtorikian
Copy link
Owner

Oh, that's really interesting. In all my years of Ruby I never knew that was suggested.

But, what if the code in the project is written using some new Ruby syntax, that the contributor doesn't have installed? In other words, how would I communicate to someone else that "Ruby 3.x.y is recommended to work with this project"?

@w-masahiro-ct
Copy link
Contributor Author

w-masahiro-ct commented Dec 30, 2024

How about using TargetRubyVersion in .rubocop.yml?
I just commited now.
Minimum ruby version of this repository is 3.1, so I specified 3.1 in .rubocop.yml.

For reference, below is the .rubocop.yml of Faraday.
https://github.com/lostisland/faraday/blob/a9cf00425e3abc99b78952af44deb2912a65a882/.rubocop.yml#L10
Minimum ruby version of Faraday is 3.0.
https://github.com/lostisland/faraday/blob/a9cf00425e3abc99b78952af44deb2912a65a882/faraday.gemspec#L16
I think it's also good to write minimum ruby version directly in README.md.
https://github.com/lostisland/faraday?tab=readme-ov-file#supported-ruby-versions

@gjtorikian
Copy link
Owner

Yeah...Faraday has over 5,000 stars and hundreds of millions of downloads. I consider this (and most of my projects) to be repositories that meet my needs, and my tooling. If the contribution and usage pool increases to the point where such formal versioning statements are needed, that's great.

But for now, I like having all my projects formatted the same way (even if they're "wrong") so I have one less thing to think about. ✌️ In any event, it doesn't look like the lack of such versioning prevented earlier contributions, so I do not think this should be done. Thank you for your help and feedback though!

@w-masahiro-ct
Copy link
Contributor Author

w-masahiro-ct commented Jan 5, 2025

@gjtorikian Sorry for reopening PR #45.
I only added the matrix to test CI.
If you also oppose adding the matrix, feel free to close PR #45.
However, in my opinion, it is very important to have a matrix.

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