search

dblock / iex-ruby-client

IEX Finance API Ruby Client
MIT License
118 stars 60 forks source link

Update to maintained Ruby version 2.7.6 #114

Closed agrberg closed 2 years ago

agrberg commented 2 years ago

~Built on top of an intended to be merged after https://github.com/dblock/iex-ruby-client/pull/113~

Now built on top of https://github.com/dblock/iex-ruby-client/pull/117 to restore CI and then update to a maintained Ruby.

resolves #115

agrberg commented 2 years ago

I don't see many benefits to making Ruby 2.7 the minimum. Maybe we can keep it at 2.4? 2.6? for now? Either way we need CI back to see what's possible #116

Good call regarding CI. It wasn't straightforward but it ended up being some minimal changes. I rebased this on #117 and split up the commits into an incredibly minimal 3f9cc20 to update the Ruby version. After this, I'd agree it would be good enough to keep on Ruby 2.4.

Now e8fc984 contains the safer changes and rubocop todo updates as well as additional current locks on the dev dependencies.

Edit: Updated the commits post rebase of #117 to keep things fresh and ready

dblock commented 2 years ago

Thanks so much for all of this! Let's get the CI to pass and cleanup the rest of the PRs.

dblock commented 2 years ago

Rebase this one and let's re-review?

agrberg commented 2 years ago

This one is ready but it still depends on the question of whether or not we want to restrict the library to only maintained versions of Ruby. The Rubocop updates are nice but hardly required.

FWIW It looks like the token I copied from your Slack client project doesn't have write access to the project. While the output is not familiar, it appears to be successful and simply unable to state that it is successful via a PR comment.

dblock commented 2 years ago

This one is ready but it still depends on the question of whether or not we want to restrict the library to only maintained versions of Ruby. The Rubocop updates are nice but hardly required.

I think I'd rather not restrict the library unnecessarily just to make rubocop happy, it's bound to make someone unhappy, I'm still on 2.6.x for example in at least one project that uses this. WDYT?

I think CI should only run rubocop once (which can be on ruby >= 2.7), the rubocop target version can be set to 2.4, and version-specific CI would run the range of supported rubies via rake spec?

FWIW It looks like the token I copied from your Slack client project doesn't have write access to the project. While the output is not familiar, it appears to be successful and simply unable to state that it is successful via a PR comment.

I opened https://github.com/dblock/iex-ruby-client/issues/118, we can deal with it later. Danger isn't supposed to need write access, it just needs to be able to comment on the PRs, which is something any account (and therefore this super limited PAT) can do.

agrberg commented 2 years ago

I agree. Going to close this. We'll revisit minimum Ruby version if there's a forward facing problem in the future like all the kwarg changes for Ruby 3.