-
Notifications
You must be signed in to change notification settings - Fork 112
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
Get actions working again #133
Get actions working again #133
Conversation
b753578
to
db9be59
Compare
@fgrehm It looks like maybe a Codecov.io upload token is missing from the GitHub Secrets, so the Action is failing. Is that something you have, or can add? Or should we just turn off Codecov? |
db9be59
to
d58d440
Compare
Lets remove it for now! |
Which actually works with newer Rubies
It's broken on CI due to missing API Keys. If we want to bring this back, fine, but whoever does can be sure to set up the necessary keys and services.
d4d372c
to
035c1cb
Compare
strategy: | ||
matrix: | ||
ruby-version: [2.7, 3.0, truffleruby-head] | ||
ruby-version: | ||
- "2.7" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should only keep versions that are still supported (AKA 3.1+ as per https://endoflife.date/ruby)
Maybe also get rid of truffleruby? Can't remember if it was something someone requested compatibility for or if I added just to try and see if it was compatible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm fine to drop older Rubies an EoL'd dependencies in general. Maybe we can do all of that as part of the next major version bump, and include some of the other PRs in it too.
@@ -17,18 +17,12 @@ Gem::Specification.new do |gem| | |||
|
|||
gem.files = `git ls-files`.split($INPUT_RECORD_SEPARATOR) | |||
gem.executables = gem.files.grep(%r{^exe/}).map { |f| File.basename(f) } | |||
gem.test_files = gem.files.grep(%r{^(test|spec|features)/}) | |||
gem.require_paths = ['lib'] | |||
|
|||
gem.add_dependency 'actionmailer', '>= 5.2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do u know if 5.x still works with the gem? I wouldn't mind bumping the dep to 6.x or 7.x if it gets into our way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. I've been using 6.1 since that's what's in the Gemfile
, IIRC?
@@ -17,18 +17,12 @@ Gem::Specification.new do |gem| | |||
|
|||
gem.files = `git ls-files`.split($INPUT_RECORD_SEPARATOR) | |||
gem.executables = gem.files.grep(%r{^exe/}).map { |f| File.basename(f) } | |||
gem.test_files = gem.files.grep(%r{^(test|spec|features)/}) | |||
gem.require_paths = ['lib'] | |||
|
|||
gem.add_dependency 'actionmailer', '>= 5.2' | |||
gem.add_dependency 'letter_opener', '~> 1.7' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know. Now that this PR has CI ✅ again, I'm thinking maybe I'll merge it. Then take a pass at dropping older Rubies, updating minimum dependencies, etc… in another PR. Just to keep the PRs smaller and easier to "see" the deltas, if nothing else.
Run them as separate steps so we see which one actually fails. This also makes Rakefile non-executable. Unsure why it was to start with, but it was making Truffle Ruby freak out.
It turns out, we weren't actually using them! Maybe left over from some past implementation?
No description provided.