zinc-collective / compensated

Create value. Get Paid.
Other
13 stars 1 forks source link

Updating Ruby version within bin/setup-matrix. #81

Closed cjoulain closed 4 years ago

cjoulain commented 4 years ago

I ran some bin executables within this repo and found that, within the bin/setup-matrix script, the ActiveSupport gem requires a higher version of Ruby. Currently, it's listed as 2.4.9. Do you know how to gauge which version of Ruby to bump it up to? Is it by the current stable version as determined by Ruby maintainers or are there other factors at play?

Since it seems like a minor change, I could try to tackle that but wanted to see what the process for the change (if accepted) would be.

Screen Shot 2020-06-15 at 1 45 31 PM
zspencer commented 4 years ago

Hey @cjoulain, this is a great question!

We may want to create a section in the README to indicate which versions of Ruby we intend to support, one in compensated-ruby to indicate which versions of Ruby the library works with, and one in compensated-rails to test which versions of Ruby we want to test with which versions of Rails.

From a Ruby perspective, I believe we at a minimum want to support the versions of Ruby that are in Normal and Security Maintenance.

How do you feel about the following support matrix for compensated-rails Ruby Rails Test
2.3 4
2.3 5
2.3 6
2.4 4
2.4 5
2.4 6
2.5 4
2.5 5
2.5 6
2.6 4
2.6 5
2.6 6
2.7 4
2.7 5
2.7 6
zspencer commented 4 years ago

Oddly, I'm having trouble re-producing this. I blew away my Gemfile.lock in Compensated-Ruby, because I expected it to want to pull in the latest ActiveSupport. However it did not.

I think the test-matrix was a reasonable scaffold in the short-term for getting the system working; and for most folks running bin/setup and bin/test should be enough to get them rolling.

That said, I think maybe we want to sprout a features/supported-languages.feature that describes what ruby version each module (compensated-ruby, compensated-proxy, compensated-rails) supports and runs the setup + test on them.

zspencer commented 4 years ago

@cjoulain - I'm thinking that we want to reframe this ticket to something that's a bit more actionable, for example:

Title: Client knows which Ruby versions compensated works with

Body:
```features/ruby-language-support.feature
Feature: Ruby Language Support
  In order to be confident Compensated will work with my project
  I want to know which version of the Ruby language each compensated module works with

  Scenario: Client knows which versions of Ruby `compensated-ruby` works with
  Scenario: Client knows which versions of Ruby and Rails `compensated-rails` works with
  Scenario: Client knows which versions of Ruby `compensated-proxy` runs in

This Issue May Be Closed When

labels: documentation, test, code



Ideally, individual contributors will never have to run the full matrix; as CI runs all the tests across every specified ruby version already. Instead, individual contributors will want to run `bin/setup` and `bin/test` and it will use whatever their current ruby version is. You can see some past conversations about this in @user512's pull request where he initially sprouted the test-matrix scripts https://github.com/zinc-collective/compensated/pull/48
user512 commented 4 years ago

Activesupport 6 is only required in compensated-rails and I haven't setup test-matrix on it yet so it shouldn't have to install any Rails 6 related gem.

I noticed there's a cross in your command prompt which might indicate some code was changed locally, do you happen to know if there's a change locally?

zspencer commented 4 years ago

@cjoulain - Now that I've updated the README to indicate that we don't expect contributors to be running the tests against all the different ruby versions, as well as documented publicly the ruby versions we support for each package, does this resolve your issue?

If so, please close it; or let me know what else is needed to get you feeling comfortable running the tests locally.

cjoulain commented 4 years ago

Re-ran bin/setup and bin/test and the recent change resolves my issue! Closing now. Thanks, Zee and Tom.