Top level should be module not class

[ioquatix]

Hello

The top level of your gem should be a module, not a class, to follow standard conventions.

[rdp]

PR's welcome! :)

…

On Tue, May 16, 2017 at 8:22 PM, Samuel Williams ***@***.***> wrote: Hello The top level of your gem should be a module, not a class, to follow standard conventions. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#27>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAw0MlLKM1QEg04XGMkj5c5e6t-kC1iks5r6lnMgaJpZM4NdS_l> .

[pboling]

@ioquatix is correct, a class at the top-level can cause a number of problems, and is against standard convention for good reasons. I've made the mistake before, and vowed never to repeat it.

Reverse Dependencies

This gem has many reverse dependencies, totalling hundreds of millions of downloads, and, given the nature of software, many of those mature, stable packages would have little reason to be updated other than to pull in a major version upgrade to this gem.

This gem should not make a change from top-level class to top-level module without a major version bump, as it would be an "incompatible API change" IMO. If it were to happen it would result in "chained upgrade hell", fracturing the Ruby ecosystem a bit like is happening right now between Faraday v1 and Faraday v2.

A Version Split?

In a version split a project's dependencies either need to all be updated to work with v.<next>, or they all have to remain stuck at v.<current> (forever). Many dependencies are not being maintained any more which has resulted in a great deal of effort expended on the part of too few people.

As a taste of why a version split outcome is undesireable, here are some projects affected by the Faraday version split: elasticsearch, discourse, oauth2, danger, and all their dependencies. Danger, used by thousands of projects, is at risk, as many major projects are removing it so they can upgrade to Faraday 2, including Github's oktokit and Slack's ruby client.

What is the state of this library?

As an outside observer who has never before interacted with it, or even used it...

• The author/maintainer hasn't changed much in years, which, as I argued at the beginning, may be for the best!
• Deep reverse dependency tree, and changes have an extensive ripple effect
• Adopt RubyGems best practices #58 - Hasn't kept up with latest Rubygem best practices
• Configure Github Actions #57 - There are no integrations configured for CI, or any other code analysis tools
• Have an OS::VERSION constant #61 - Missing OS::VERSION constant
• Specify required_ruby_version #62 - Missing spec.required_ruby_version in gemspec
• Adopt gem-release instead of Jeweler for releases #56 - It is packaged and released by Jeweler, a gem packaging tool that died many years ago due to being seriously broken, as evidenced by the hilarious comment in this gem's Rakefile

# HOW TO DEPLOY
# bump VERSION file manually
# rake release
# Don't forget to run rake gemspec with each release! ... I think...or rake release might be same thing
# then manually edit os.gemspec remove duplicatee rspecs, circular dependency on os? HUH?
# # then  gem build os.gemspec
# rake build doesn't work???
# sooo...
# ...gem push os-1.1.2.gem

• Current master can't do bundle install #55 - Current master can't do bundle install:

❯ bundle install
Fetching bundler 2.1.4
Installing bundler 2.1.4
Fetching gem metadata from http://rubygems.org/.......
Resolving dependencies...
Your bundle requires gems that depend on each other, creating an infinite loop. Please remove gem 'os' and try again.

• Spec don't pass on MacOS #60 - Test suite doesn't pass on MacOS

..................F..F..............

Failures:

  1) OS has working cpu count method
     Failure/Error: assert (cpu_count & (cpu_count - 1)) == 0 # CPU count is normally a power of 2
     Test::Unit::AssertionFailedError:
       <false> is not true.
     # ./spec/os_spec.rb:124:in `block (2 levels) in <top (required)>'

  2) OS should provide a path to directory for application config
     Failure/Error: assert OS.app_config_path('appname') == '/home/xdg/Library/Application Support/appname'
     Test::Unit::AssertionFailedError:
       <false> is not true.
     # ./spec/os_spec.rb:150:in `block (2 levels) in <top (required)>'

Finished in 0.03682 seconds
36 examples, 2 failures

• Upgrade to RSpec 3 #67 - Test suite hasn't been upgraded to RSpec 3

An error occurred while loading ./spec/os_spec.rb.
Failure/Error: config.expect_with :rspec, :stdlib # enable `should` OR `assert`

ArgumentError:
  :stdlib is not supported
# ./spec/spec_helper.rb:8:in `block in <top (required)>'
# ./spec/spec_helper.rb:7:in `<top (required)>'
# ./spec/os_spec.rb:1:in `require'
# ./spec/os_spec.rb:1:in `<top (required)>'

• Migrate autotest => guard-rspec #70 - Test suite needs to be upgraded to modern rspec-autotest

Path Forward

Given the state of this library overall I think the best route forward would be to create a new gem with the same API and funtionality, but with a new name and namespace, so that unmaintained libraries that rely on os continue to work without introducing a "chained upgrade hell".

To that end, I have forked this library, renamed the fork, and will soon release a new gem called moss (Mossy Operating System Stuff) with a top-level namespace module of Moss, which provides the same API as this gem. It will be based on the modern bundler gem template, use github actions, integrate the stability fixes from #48, and have code coverage reporting built in to pull requests, etc.

Because it will be a new gem and a new namespace, the two gems can be used side by side with no problems! No one is forced to upgrade anything, and the ecosystem remains intact!

Prior to making the fork forky though (the namespace change) I'll do upgrades that I can PR upstream to this repo, so they both benefit!

I will upstream fixes for:

• Current master can't do bundle install #55 fixed by 🚑️ Remove circular dependency, fixes #55, #60 #59
• Adopt gem-release instead of Jeweler for releases #56
• Configure Github Actions #57
• Adopt RubyGems best practices #58
• Spec don't pass on MacOS #60 fixed by 🚑️ Remove circular dependency, fixes #55, #60 #59
• Have an OS::VERSION constant #61
• Specify required_ruby_version #62
• Update License Copyright Years #63
• Rakefile best practices; Stop requiring rubygems #64 fixed by Rakefile Best Practices (& RuboCop) #72
• Upgrade to RSpec 3 #67 fixed by Upgrade RSpec to latest (including syntax) #68
• Migrate autotest => guard-rspec #70 fixed by Migrate autotest => guard-rspec #71

Q & A

Question: Why am I doing this if I have never before used this library?

I am working on upgrading my rspec-pending_for gem, and need the functionality this library provides, and will probably want to extend the functionality. Given that the maintainer may not want my extensions, the trouble caused by churn if the issues were to be addressed, and the actual current issues it seemed best to hard fork. - @pboling, here, now

Question: Why moss?

I considered oss, but it has a strong association with Open Source Software already. I considered os2, but google searches for os2 in the computer programming context are saturated by IBM's eponymous product. So aping on the stolid linux tradition of package names that are self-referencing, I chose moss. - @pboling, here, now

Interested to hear what @rdp thinks about all this.

[ioquatix]

I think forking should almost always be a last resort.

[pboling]

I normally agree, but in this case it might be quite disruptive to fix the top-level namespace. What I've been dealing with on the faraday version split is real bad. I maintain the oauth2 gem, and I ended up having to remove danger in order to support faraday v2. oauth2 has a huge reverse dependency tree as well. It was a mess.

On the other hand, when factory_girl became factory_bot, a hard fork, it was easy.

[ioquatix]

Yeah fair enough.

[pboling]

I'd be happy to PR the hard fork (by which I mean a namespace change and gem name change) into this repo if @rdp agrees to change both. Then it would be just like the factory_girl/bot rename. Alternatively, or in addition, would be happy to take over maintenance.