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

Travis build change: Fix build, use latest JRuby #435

Merged
merged 3 commits into from
May 11, 2017
Merged

Travis build change: Fix build, use latest JRuby #435

merged 3 commits into from
May 11, 2017

Conversation

olleolleolle
Copy link
Contributor

@olleolleolle olleolleolle commented Dec 6, 2016

...while using the "let Bundler know which Ruby version we are currently using, so it can locate fitting gems on its own" ruby version annotation in the Gemfile.

This sweeping change:

  • also moves all the dev dependencies needed into a Bundler group
  • And ensures a modern-enough Bundler, so that the above method works.
  • Works around a Rubocop version issue: lint only on a Ruby 2.4.1 with latest Rubocop

@olleolleolle olleolleolle changed the title Travis build change: rely on Bundler finding gems Travis build change: Fix build, use latest JRuby Mar 17, 2017
  - Lint only 1 time, on modern Ruby
@olleolleolle
Copy link
Contributor Author

@jnicklas Here! A Friday present: this should build green.

@olleolleolle
Copy link
Contributor Author

olleolleolle commented Apr 5, 2017

Oh, TODO: Do not fail Rubocop on 2.4.1. ✅

@jaredbeck
Copy link

Olle, if you're done working on this, I can pick up where you left off. However, I'm concerned that there aren't any maintainers in the discussion here. Can a maintainer please confirm that they will review a PR that fixes CI? CC: @jnicklas @thomasklemm

@olleolleolle
Copy link
Contributor Author

@jaredbeck Thank you for looking at this.

It made me lint the last bit and go green.

I think this is as ready is it will get. Can we get it merged?

@ce07c3
Copy link

ce07c3 commented May 11, 2017

@jaredbeck @olleolleolle it's been abandoned more or less, maintainers are active twice a year. I'd suggest starting a new repo and releasing under a different gem name, e.g. pundit2.

@thomasklemm
Copy link
Collaborator

Hey guys, wanted to post this week anyway. Sorry for letting this drag on a bit.

I'd like to hand over my spot as maintainer to someone else who is more active, since I'm too far away from the codebase to help at the moment. Will open another issue after talking with @jnicklas for finding a replacement maintainer for myself.

@@ -19,12 +20,4 @@ Gem::Specification.new do |gem|
gem.require_paths = ["lib"]

gem.add_dependency "activesupport", ">= 3.0.0"
gem.add_development_dependency "activemodel", ">= 3.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olleolleolle This PR looks solid, thanks a lot.

Could you reiterate on these lines, a hint on why we can remove these development dependencies would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomasklemm Oh, since all development takes place within the Bundler context, and we have better control of versions needed etc using Gemfiles, we can use a Group in Bundler (:development) and keep development-related things there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bundler's version-and-platform finding mechanism finds a gem version that suits the current running Ruby.

In order to give more background, here is an example.

The Bundler mechanism assists us when it comes to secondary deps: let's say a test tool that we explicitly refer to has a dependency. We get that pulled in implicitly. That dep needs to be "> 2" on Ruby 2+ and "< 2" on Ruby 1.9. If we're unlucky and have mentioned all this in the gemspec, we have to find and keep updated a least common denominator and that could be tricky.

(Status: not enough coffee, not good at story-telling right now.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I see, thanks for the quick answer!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olleolleolle I'm doing some work on the gem and trying to follow this thread. Can you explain how the ruby version example you mention above is any different when specifying gems in the Gemfile? My understanding is that a Gemfile can only specify one ruby version.

Also, while we want the gem to be compatible with as many ruby versions as possible, for the purposes of development of the gem I'm much more open to supporting a smaller set of ruby versions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olleolleolle I totally agree that libraries in usage want to support many ruby versions, but is the same true for libraries in development? Note that all the gems we're talking about here are development dependencies, so they should have no impact at all on users of the gem, right? Only on developers of the gem.

Again, the docs that I linked to seem to be Bundler themselves specifically saying that Gemfile.lock should be checked in for gems, since it only impacts gem maintainers, not gem users. Or am I reading it wrong?

(Thanks for taking the time to go down this rabbithole with me by the way - I really appreciate it)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a better link: https://bundler.io/man/bundle-install.1.html#THE-GEMFILE-LOCK

When Bundler first shipped, the Gemfile.lock was included in the .gitignore file included with generated gems. Over time, however, it became clear that this practice forces the pain of broken dependencies onto new contributors, while leaving existing contributors potentially unaware of the problem. Since bundle install is usually the first step towards a contribution, the pain of broken dependencies would discourage new contributors from contributing. As a result, we have revised our guidance for gem authors to now recommend checking in the lock for gems.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rabbit-hole expands! Thanks for taking me to task and re-read the section.

Recommended praxis has shifted & been documented!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olleolleolle I'm not sure I understand your last comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for telling me about the updated documentation! I wasn't aware.

@thomasklemm thomasklemm mentioned this pull request May 11, 2017
@ce07c3
Copy link

ce07c3 commented May 11, 2017

@thomasklemm thanks for responding, I'd be sad if this project were to become obsolete because it wasn't maintained. :)

Copy link
Collaborator

@thomasklemm thomasklemm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR, thanks a lot for your efforts 👍

@thomasklemm thomasklemm merged commit ac2a25d into varvet:master May 11, 2017
@olleolleolle olleolleolle deleted the fix/build-travis branch May 11, 2017 13:05
@olleolleolle
Copy link
Contributor Author

Thanks 😻

dgmstuart added a commit that referenced this pull request Aug 5, 2019
This commit does three things:

1. Move development dependencies back to gemspec
2. Restrict versions of some gems
3. Commit Gemfile.lock

Each of these requires a more detailed discussion:

1. Move development dependencies back to gemspec

9691355 originally moved all these dependencies into the Gemfile:

#435

The recommendation in the docs seems to be to use the gemspec for
development dependencies:

https://bundler.io/guides/creating_gem.html#getting-started

2. Restrict versions of some gems

While we want to be as flexible as possible with the gem's production
dependencies, we need to be more specific with our development
dependencies. In particular gems developed against one version of
Rubocop cannot be expected to work with later versions, since Rubocop
adds and renames cops in new versions.

In particular version 0.70.0 renames a cop which we are specifying,
resulting in the following errors.

    .rubocop.yml:81: `Style/TrivialAccessors` is concealed by line 96
    Error: The `Layout/IndentHash` cop has been renamed to `Layout/IndentFirstHashElement`.
    (obsolete configuration found in .rubocop.yml, please update it)

For gems whose versions we don't care about, we can specify this
with "~> 0" to allow any version. For other gems where the version is
likely to be significant we can specify a version range.

Note that when running `gem build pundit.gemspec` we get warnings about
some open-ended dependencies:

    WARNING:  open-ended dependency on activesupport (>= 3.0.0) is not recommended
    if activesupport is semantically versioned, use:
      add_runtime_dependency 'activesupport', '~> 3.0', '>= 3.0.0'

However, I feel OK about these being loose, and since this gem is not so
often maintained, I'd prefer not to place an upper limit on the
runtime_dependency. The other gems with open-ended dependencies are also
part of Rails, so I believe they'll need to be the same version as
activesupport, which is why they have the same version constraint.

3. Commit Gemfile.lock

When this gem was created the standard practice was to ignore the
Gemfile.lock, but the guidance has changed:

> When Bundler first shipped, the Gemfile.lock was included in the
> .gitignore file included with generated gems. Over time, however, it
> became clear that this practice forces the pain of broken dependencies
> onto new contributors, while leaving existing contributors potentially
> unaware of the problem. Since bundle install is usually the first step
> towards a contribution, the pain of broken dependencies would
> discourage new contributors from contributing. As a result, we have
> revised our guidance for gem authors to now recommend checking in the
> lock for gems.

https://bundler.io/man/bundle-install.1.html#THE-GEMFILE-LOCK

By committing the Gemfile.lock we ensure that all developers of this gem
have the same dependencies.
dgmstuart added a commit that referenced this pull request Aug 5, 2019
This commit does three things:

1. Move development dependencies back to gemspec
2. Restrict versions of some gems
3. Commit Gemfile.lock

Each of these requires a more detailed discussion:

1. Move development dependencies back to gemspec

9691355 originally moved all these dependencies into the Gemfile:

#435

The recommendation in the docs seems to be to use the gemspec for
development dependencies:

https://bundler.io/guides/creating_gem.html#getting-started

2. Restrict versions of some gems

While we want to be as flexible as possible with the gem's production
dependencies, we need to be more specific with our development
dependencies. In particular gems developed against one version of
Rubocop cannot be expected to work with later versions, since Rubocop
adds and renames cops in new versions.

In particular version 0.70.0 renames a cop which we are specifying,
resulting in the following errors.

    .rubocop.yml:81: `Style/TrivialAccessors` is concealed by line 96
    Error: The `Layout/IndentHash` cop has been renamed to `Layout/IndentFirstHashElement`.
    (obsolete configuration found in .rubocop.yml, please update it)

Note that when running `gem build pundit.gemspec` we get warnings about
some open-ended dependencies:

    WARNING:  open-ended dependency on activesupport (>= 3.0.0) is not recommended
    if activesupport is semantically versioned, use:
      add_runtime_dependency 'activesupport', '~> 3.0', '>= 3.0.0'

However, I feel OK about these being loose, and since this gem is not so
often maintained, I'd prefer not to place an upper limit on the
runtime_dependency. The other gems with open-ended dependencies are also
part of Rails, so I believe they'll need to be the same version as
activesupport, which is why they have the same version constraint.

For the gems where we don't specify any version constraint we also get
this warning. For most of these (eg. pry) we genuinely don't care what
version is used. For Bundler, a different version will be used on CI
than locally, and specifying a version seems to conflict when running
`bundle install` with a different version, so this feels like it should
be left blank.

3. Commit Gemfile.lock

When this gem was created the standard practice was to ignore the
Gemfile.lock, but the guidance has changed:

> When Bundler first shipped, the Gemfile.lock was included in the
> .gitignore file included with generated gems. Over time, however, it
> became clear that this practice forces the pain of broken dependencies
> onto new contributors, while leaving existing contributors potentially
> unaware of the problem. Since bundle install is usually the first step
> towards a contribution, the pain of broken dependencies would
> discourage new contributors from contributing. As a result, we have
> revised our guidance for gem authors to now recommend checking in the
> lock for gems.

https://bundler.io/man/bundle-install.1.html#THE-GEMFILE-LOCK

By committing the Gemfile.lock we ensure that all developers of this gem
have the same dependencies.
dgmstuart added a commit that referenced this pull request Aug 5, 2019
This commit does three things:

1. Move development dependencies back to gemspec
2. Restrict versions of some gems
3. Commit Gemfile.lock

Each of these requires a more detailed discussion:

1. Move development dependencies back to gemspec

9691355 originally moved all these dependencies into the Gemfile:

#435

The recommendation in the docs seems to be to use the gemspec for
development dependencies:

https://bundler.io/guides/creating_gem.html#getting-started

2. Restrict versions of some gems

While we want to be as flexible as possible with the gem's production
dependencies, we need to be more specific with our development
dependencies. In particular gems developed against one version of
Rubocop cannot be expected to work with later versions, since Rubocop
adds and renames cops in new versions.

In particular version 0.70.0 renames a cop which we are specifying,
resulting in the following errors.

    .rubocop.yml:81: `Style/TrivialAccessors` is concealed by line 96
    Error: The `Layout/IndentHash` cop has been renamed to `Layout/IndentFirstHashElement`.
    (obsolete configuration found in .rubocop.yml, please update it)

Note that when running `gem build pundit.gemspec` we get warnings about
some open-ended dependencies:

    WARNING:  open-ended dependency on activesupport (>= 3.0.0) is not recommended
    if activesupport is semantically versioned, use:
      add_runtime_dependency 'activesupport', '~> 3.0', '>= 3.0.0'

However, I feel OK about these being loose, and since this gem is not so
often maintained, I'd prefer not to place an upper limit on the
runtime_dependency. The other gems with open-ended dependencies are also
part of Rails, so I believe they'll need to be the same version as
activesupport, which is why they have the same version constraint.

For the gems where we don't specify any version constraint we also get
this warning. For most of these (eg. pry) we genuinely don't care what
version is used.

For Bundler, we can't use a version in the 2.x.x range without removing
support for ruby 2.1 and 2.2, so we lock this to the specific version
we're using on CI.

3. Commit Gemfile.lock

When this gem was created the standard practice was to ignore the
Gemfile.lock, but the guidance has changed:

> When Bundler first shipped, the Gemfile.lock was included in the
> .gitignore file included with generated gems. Over time, however, it
> became clear that this practice forces the pain of broken dependencies
> onto new contributors, while leaving existing contributors potentially
> unaware of the problem. Since bundle install is usually the first step
> towards a contribution, the pain of broken dependencies would
> discourage new contributors from contributing. As a result, we have
> revised our guidance for gem authors to now recommend checking in the
> lock for gems.

https://bundler.io/man/bundle-install.1.html#THE-GEMFILE-LOCK

By committing the Gemfile.lock we ensure that all developers of this gem
have the same dependencies.

In order to ensure that this can work on CI (which uses a specific
version of bundler for older rubies), we need to edit the Gemfile.lock
to pretend that it was bundled with 1.17.3
dgmstuart added a commit that referenced this pull request Aug 5, 2019
This commit does two things:

1. Move development dependencies back to gemspec
2. Restrict versions of some gems

It should also be noted that the recommendation is to commit
Gemfile.lock, however in this case it is not possible to do so:

3. Why we don't commit Gemfile.lock

Each of these requires a more detailed discussion:

1. Move development dependencies back to gemspec

9691355 originally moved all these dependencies into the Gemfile:

#435

The recommendation in the docs seems to be to use the gemspec for
development dependencies:

https://bundler.io/guides/creating_gem.html#getting-started

2. Restrict versions of some gems

While we want to be as flexible as possible with the gem's production
dependencies, we need to be more specific with our development
dependencies. In particular gems developed against one version of
Rubocop cannot be expected to work with later versions, since Rubocop
adds and renames cops in new versions.

In particular version 0.70.0 renames a cop which we are specifying,
resulting in the following errors.

    .rubocop.yml:81: `Style/TrivialAccessors` is concealed by line 96
    Error: The `Layout/IndentHash` cop has been renamed to `Layout/IndentFirstHashElement`.
    (obsolete configuration found in .rubocop.yml, please update it)

Note that when running `gem build pundit.gemspec` we get warnings about
some open-ended dependencies:

    WARNING:  open-ended dependency on activesupport (>= 3.0.0) is not recommended
    if activesupport is semantically versioned, use:
      add_runtime_dependency 'activesupport', '~> 3.0', '>= 3.0.0'

However, I feel OK about these being loose, and since this gem is not so
often maintained, I'd prefer not to place an upper limit on the
runtime_dependency. The other gems with open-ended dependencies are also
part of Rails, so I believe they'll need to be the same version as
activesupport, which is why they have the same version constraint.

For the gems where we don't specify any version constraint we also get
this warning. For most of these (eg. pry) we genuinely don't care what
version is used.

3. Commit Gemfile.lock

When this gem was created the standard practice was to gitignore the
Gemfile.lock, but the guidance has changed:

> When Bundler first shipped, the Gemfile.lock was included in the
> .gitignore file included with generated gems. Over time, however, it
> became clear that this practice forces the pain of broken dependencies
> onto new contributors, while leaving existing contributors potentially
> unaware of the problem. Since bundle install is usually the first step
> towards a contribution, the pain of broken dependencies would
> discourage new contributors from contributing. As a result, we have
> revised our guidance for gem authors to now recommend checking in the
> lock for gems.

https://bundler.io/man/bundle-install.1.html#THE-GEMFILE-LOCK

The intention is that by committing the Gemfile.lock we ensure that all
developers of this gem have the same dependencies.

HOWEVER, we are currently supporting ruby 2.1 and 2.2. Newer versions of
some gems (eg. i18n, which is required by activesupport) are only
compatible with ruby 2.3 and up, so a lockfile compatible with the
latest version of activesupport (which is a runtime dependency) will not
be compatible with these versions.

The impact of this is that we can't commit the lockfile without also
dropping support for versions 2.1 and 2.2.
dgmstuart added a commit that referenced this pull request Aug 5, 2019
This commit does two things:

1. Move development dependencies back to gemspec
2. Restrict versions of some gems

It should also be noted that the recommendation is to commit
Gemfile.lock, however in this case it is not possible to do so:

3. Why we don't commit Gemfile.lock

Each of these requires a more detailed discussion:

1. Move development dependencies back to gemspec

9691355 originally moved all these dependencies into the Gemfile:

#435

The recommendation in the docs seems to be to use the gemspec for
development dependencies:

https://bundler.io/guides/creating_gem.html#getting-started

2. Restrict versions of some gems

While we want to be as flexible as possible with the gem's production
dependencies, we need to be more specific with our development
dependencies. In particular gems developed against one version of
Rubocop cannot be expected to work with later versions, since Rubocop
adds and renames cops in new versions.

In particular version 0.70.0 renames a cop which we are specifying,
resulting in the following errors.

    .rubocop.yml:81: `Style/TrivialAccessors` is concealed by line 96
    Error: The `Layout/IndentHash` cop has been renamed to `Layout/IndentFirstHashElement`.
    (obsolete configuration found in .rubocop.yml, please update it)

Note that when running `gem build pundit.gemspec` we get warnings about
some open-ended dependencies:

    WARNING:  open-ended dependency on activesupport (>= 3.0.0) is not recommended
    if activesupport is semantically versioned, use:
      add_runtime_dependency 'activesupport', '~> 3.0', '>= 3.0.0'

However, I feel OK about these being loose, and since this gem is not so
often maintained, I'd prefer not to place an upper limit on the
runtime_dependency. The other gems with open-ended dependencies are also
part of Rails, so I believe they'll need to be the same version as
activesupport, which is why they have the same version constraint.

For the gems where we don't specify any version constraint we also get
this warning. For most of these (eg. pry) we genuinely don't care what
version is used.

3. Why we don't commit Gemfile.lock

When this gem was created the standard practice was to gitignore the
Gemfile.lock, but the guidance has changed:

> When Bundler first shipped, the Gemfile.lock was included in the
> .gitignore file included with generated gems. Over time, however, it
> became clear that this practice forces the pain of broken dependencies
> onto new contributors, while leaving existing contributors potentially
> unaware of the problem. Since bundle install is usually the first step
> towards a contribution, the pain of broken dependencies would
> discourage new contributors from contributing. As a result, we have
> revised our guidance for gem authors to now recommend checking in the
> lock for gems.

https://bundler.io/man/bundle-install.1.html#THE-GEMFILE-LOCK

The intention is that by committing the Gemfile.lock we ensure that all
developers of this gem have the same dependencies.

HOWEVER, we are currently supporting ruby 2.1 and 2.2. Newer versions of
some gems (eg. i18n, which is required by activesupport) are only
compatible with ruby 2.3 and up, so a lockfile compatible with the
latest version of activesupport (which is a runtime dependency) will not
be compatible with these versions.

The impact of this is that we can't commit the lockfile without also
dropping support for versions 2.1 and 2.2.
dgmstuart added a commit that referenced this pull request Aug 5, 2019
This commit does two things:

1. Move development dependencies back to gemspec
2. Restrict versions of some gems

It should also be noted that the recommendation is to commit
Gemfile.lock, however in this case it is not possible to do so:

3. Why we don't commit Gemfile.lock

Each of these requires a more detailed discussion:

1. Move development dependencies back to gemspec

9691355 originally moved all these dependencies into the Gemfile:

#435

The recommendation in the docs seems to be to use the gemspec for
development dependencies:

https://bundler.io/guides/creating_gem.html#getting-started

2. Restrict versions of some gems

While we want to be as flexible as possible with the gem's production
dependencies, we need to be more specific with our development
dependencies. In particular gems developed against one version of
Rubocop cannot be expected to work with later versions, since Rubocop
adds and renames cops in new versions.

In particular version 0.70.0 renames a cop which we are specifying,
resulting in the following errors.

    .rubocop.yml:81: `Style/TrivialAccessors` is concealed by line 96
    Error: The `Layout/IndentHash` cop has been renamed to `Layout/IndentFirstHashElement`.
    (obsolete configuration found in .rubocop.yml, please update it)

Rubocop version 0.67.2 passes without issues on the latest ruby version,
but is incompatible with 2.2.x and earlier. 0.57.2 is the latest version
which works everywhere.

Note that when running `gem build pundit.gemspec` we get warnings about
some open-ended dependencies:

    WARNING:  open-ended dependency on activesupport (>= 3.0.0) is not recommended
    if activesupport is semantically versioned, use:
      add_runtime_dependency 'activesupport', '~> 3.0', '>= 3.0.0'

However, I feel OK about these being loose, and since this gem is not so
often maintained, I'd prefer not to place an upper limit on the
runtime_dependency. The other gems with open-ended dependencies are also
part of Rails, so I believe they'll need to be the same version as
activesupport, which is why they have the same version constraint.

For the gems where we don't specify any version constraint we also get
this warning. For most of these (eg. pry) we genuinely don't care what
version is used.

3. Why we don't commit Gemfile.lock

When this gem was created the standard practice was to gitignore the
Gemfile.lock, but the guidance has changed:

> When Bundler first shipped, the Gemfile.lock was included in the
> .gitignore file included with generated gems. Over time, however, it
> became clear that this practice forces the pain of broken dependencies
> onto new contributors, while leaving existing contributors potentially
> unaware of the problem. Since bundle install is usually the first step
> towards a contribution, the pain of broken dependencies would
> discourage new contributors from contributing. As a result, we have
> revised our guidance for gem authors to now recommend checking in the
> lock for gems.

https://bundler.io/man/bundle-install.1.html#THE-GEMFILE-LOCK

The intention is that by committing the Gemfile.lock we ensure that all
developers of this gem have the same dependencies.

HOWEVER, we are currently supporting ruby 2.1 and 2.2. Newer versions of
some gems (eg. i18n, which is required by activesupport) are only
compatible with ruby 2.3 and up, so a lockfile compatible with the
latest version of activesupport (which is a runtime dependency) will not
be compatible with these versions.

The impact of this is that we can't commit the lockfile without also
dropping support for versions 2.1 and 2.2.
dgmstuart added a commit that referenced this pull request Aug 7, 2019
This commit does two things:

1. Move development dependencies back to gemspec
2. Restrict versions of some gems

It should also be noted that the recommendation is to commit
Gemfile.lock, however in this case it is not possible to do so:

3. Why we don't commit Gemfile.lock

Each of these requires a more detailed discussion:

1. Move development dependencies back to gemspec

9691355 originally moved all these dependencies into the Gemfile:

#435

The recommendation in the docs seems to be to use the gemspec for
development dependencies:

https://bundler.io/guides/creating_gem.html#getting-started

2. Restrict versions of some gems

While we want to be as flexible as possible with the gem's production
dependencies, we need to be more specific with our development
dependencies. In particular gems developed against one version of
Rubocop cannot be expected to work with later versions, since Rubocop
adds and renames cops in new versions.

In particular version 0.70.0 renames a cop which we are specifying,
resulting in the following errors.

    .rubocop.yml:81: `Style/TrivialAccessors` is concealed by line 96
    Error: The `Layout/IndentHash` cop has been renamed to `Layout/IndentFirstHashElement`.
    (obsolete configuration found in .rubocop.yml, please update it)

Rubocop version 0.67.2 passes without issues on the latest ruby version,
but is incompatible with 2.2.x and earlier. 0.57.2 is the latest version
which works everywhere.

Note that when running `gem build pundit.gemspec` we get warnings about
some open-ended dependencies:

    WARNING:  open-ended dependency on activesupport (>= 3.0.0) is not recommended
    if activesupport is semantically versioned, use:
      add_runtime_dependency 'activesupport', '~> 3.0', '>= 3.0.0'

However, I feel OK about these being loose, and since this gem is not so
often maintained, I'd prefer not to place an upper limit on the
runtime_dependency. The other gems with open-ended dependencies are also
part of Rails, so I believe they'll need to be the same version as
activesupport, which is why they have the same version constraint.

For the gems where we don't specify any version constraint we also get
this warning. For most of these (eg. pry) we genuinely don't care what
version is used.

3. Why we don't commit Gemfile.lock

When this gem was created the standard practice was to gitignore the
Gemfile.lock, but the guidance has changed:

> When Bundler first shipped, the Gemfile.lock was included in the
> .gitignore file included with generated gems. Over time, however, it
> became clear that this practice forces the pain of broken dependencies
> onto new contributors, while leaving existing contributors potentially
> unaware of the problem. Since bundle install is usually the first step
> towards a contribution, the pain of broken dependencies would
> discourage new contributors from contributing. As a result, we have
> revised our guidance for gem authors to now recommend checking in the
> lock for gems.

https://bundler.io/man/bundle-install.1.html#THE-GEMFILE-LOCK

The intention is that by committing the Gemfile.lock we ensure that all
developers of this gem have the same dependencies.

HOWEVER, we are currently supporting ruby 2.1 and 2.2. Newer versions of
some gems (eg. i18n, which is required by activesupport) are only
compatible with ruby 2.3 and up, so a lockfile compatible with the
latest version of activesupport (which is a runtime dependency) will not
be compatible with these versions.

The impact of this is that we can't commit the lockfile without also
dropping support for versions 2.1 and 2.2.
HappyDevman added a commit to HappyDevman/pundit that referenced this pull request May 5, 2021
This commit does two things:

1. Move development dependencies back to gemspec
2. Restrict versions of some gems

It should also be noted that the recommendation is to commit
Gemfile.lock, however in this case it is not possible to do so:

3. Why we don't commit Gemfile.lock

Each of these requires a more detailed discussion:

1. Move development dependencies back to gemspec

9691355 originally moved all these dependencies into the Gemfile:

varvet/pundit#435

The recommendation in the docs seems to be to use the gemspec for
development dependencies:

https://bundler.io/guides/creating_gem.html#getting-started

2. Restrict versions of some gems

While we want to be as flexible as possible with the gem's production
dependencies, we need to be more specific with our development
dependencies. In particular gems developed against one version of
Rubocop cannot be expected to work with later versions, since Rubocop
adds and renames cops in new versions.

In particular version 0.70.0 renames a cop which we are specifying,
resulting in the following errors.

    .rubocop.yml:81: `Style/TrivialAccessors` is concealed by line 96
    Error: The `Layout/IndentHash` cop has been renamed to `Layout/IndentFirstHashElement`.
    (obsolete configuration found in .rubocop.yml, please update it)

Rubocop version 0.67.2 passes without issues on the latest ruby version,
but is incompatible with 2.2.x and earlier. 0.57.2 is the latest version
which works everywhere.

Note that when running `gem build pundit.gemspec` we get warnings about
some open-ended dependencies:

    WARNING:  open-ended dependency on activesupport (>= 3.0.0) is not recommended
    if activesupport is semantically versioned, use:
      add_runtime_dependency 'activesupport', '~> 3.0', '>= 3.0.0'

However, I feel OK about these being loose, and since this gem is not so
often maintained, I'd prefer not to place an upper limit on the
runtime_dependency. The other gems with open-ended dependencies are also
part of Rails, so I believe they'll need to be the same version as
activesupport, which is why they have the same version constraint.

For the gems where we don't specify any version constraint we also get
this warning. For most of these (eg. pry) we genuinely don't care what
version is used.

3. Why we don't commit Gemfile.lock

When this gem was created the standard practice was to gitignore the
Gemfile.lock, but the guidance has changed:

> When Bundler first shipped, the Gemfile.lock was included in the
> .gitignore file included with generated gems. Over time, however, it
> became clear that this practice forces the pain of broken dependencies
> onto new contributors, while leaving existing contributors potentially
> unaware of the problem. Since bundle install is usually the first step
> towards a contribution, the pain of broken dependencies would
> discourage new contributors from contributing. As a result, we have
> revised our guidance for gem authors to now recommend checking in the
> lock for gems.

https://bundler.io/man/bundle-install.1.html#THE-GEMFILE-LOCK

The intention is that by committing the Gemfile.lock we ensure that all
developers of this gem have the same dependencies.

HOWEVER, we are currently supporting ruby 2.1 and 2.2. Newer versions of
some gems (eg. i18n, which is required by activesupport) are only
compatible with ruby 2.3 and up, so a lockfile compatible with the
latest version of activesupport (which is a runtime dependency) will not
be compatible with these versions.

The impact of this is that we can't commit the lockfile without also
dropping support for versions 2.1 and 2.2.
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.

None yet

5 participants