Conversation
* Add GHA's test config * remove TravisCI config * Update references in documentation
|
The error on Ruby 3.0 and 3.1 is because Rubocop changed how it works under the hood. I'm installing Ruby 3.0 now to see what's the minimum version of Rubocop needed to get the Ruby min 2.4 working on 3.0 before addressing the 2.7 update. |
Ruby 3.0 removed hash and kwarg coersion. Rubocop 0.75.0 is the earliest version that corrects this. Add Rubocop offenses to TODO yml. See Ruby language post at https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/. I also wrote about this at https://medium.com/building-rigup/hash-and-keyword-coercion-213425f69719.
Rubocop under Ruby >= 3.1 has a problem with this file if the library is not required. My guess has to do w/ a change constant resolution.
|
Alright! Looks like it's passing on Rubies 2.7, 3.0, and 3.1 now and can be reviewed 😌 |
There was a problem hiding this comment.
Below.
Make sure CI actually passes, see https://github.com/agrberg/iex-ruby-client/actions in your fork.
Adding Ruby 3.0 support can be done later, separately. Get a minimum amount of GHA to pass. |
|
GitHub's CI is still a little new to me but if I'm reading the workflows correctly it looks like the last push to this branch yesterday passed as did the update this morning to remove the concurrency from I can move the two commits related to Ruby 3.0 and 3.1 to another PR if you feel strongly. I wanted to point out that they're essentially behind-the-scenes trivial changes though:
Please let me know if I'm missing something obvious 😅 |
dblock
left a comment
dblock
left a comment
There was a problem hiding this comment.
We do have a Dangerfile (PR linter), bring the GHA that runs Danger as well in.
https://github.com/slack-ruby/slack-ruby-client/blob/master/.github/workflows/danger.yml
It wouldn't be such a bad idea to split up Rubocop and tests as well, avoiding running rubocop on every test, but not a must have.
A few more things below.
* Remove unneeded concurrancy * Remove RACK_ENV var
agrberg
force-pushed
the
move_ci_to_gha
branch
2 times, most recently
from
5e7925b to
297b1be
Compare
No need to lint on each version of Ruby
| danger: | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| BUNDLE_GEMFILE: ${{ github.workspace }}/Gemfile.danger |
There was a problem hiding this comment.
I copied this approach from your Slack Ruby client along with a separate Gemfile.danger. I didn't pull the common items out of the Gemfile but I think it would be valuable to do later while also moving the dev dependencies from the gemspec to the Gemfile.
| steps: | ||
| - uses: actions/checkout@v2 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
It is my understanding that this will fetch all the commits as opposed to simply the last one. I'm not familiar with Danger though so I'm unsure why this is valuable. Can you illuminate?
There was a problem hiding this comment.
I forget why that was needed, but it's fetch all history for all tags and branches. I think there was some error from Danger when it didn't have access to all of it.
|
Thanks for the patience and tips. I believe the Danger workflow isn't kicking off because it is only done when making a PR as opposed to updating a PR. GHA's UI isn't letting me run in manually either. My assumption there is because it's new and hasn't ever been run. Kind of a catch-22 🥲 |
|
Merged! Let's see those other PRs. |
2 participants
resolves #116