search

rdp / os

The OS gem allows for some easy telling if you’re on windows or not. OS.windows? as well as some other helper utilities
MIT License
145 stars 33 forks source link

🚑️ Remove circular dependency, fixes #55, #60 #59

Closed pboling closed 1 year ago

pboling commented 2 years ago

Fixes #55 & #60

Signed-off-by: Peter Boling peter.boling@gmail.com

pboling commented 2 years ago

@rdp Ping!

rdp commented 2 years ago

Do the specs work with 3+ ?

On Mon, Sep 12, 2022 at 2:39 PM Peter Boling @.***> wrote:

@.**** commented on this pull request.

@rdp https://github.com/rdp

In os.gemspec https://github.com/rdp/os/pull/59#discussion_r968909214:

@@ -46,18 +46,15 @@ Gem::Specification.new do |s| s.specification_version = 4

 if Gem::Version.new(Gem::VERSION) >= Gem::Version.new('1.2.0') then
  • s.add_runtime_dependency(%q.freeze, [">= 0"])

The os gem can't depend on itself. I think this is part of the messy legacy of jeweler.

In Gemfile.lock https://github.com/rdp/os/pull/59#discussion_r968910012:

rake (~> 0.8)

  • rspec (~> 2.5.0)
  • rspec (>= 2.0)

Loosening here will allow using the transpec tool to automatically upgrade the RSpec syntax to RSpec 3+.

— Reply to this email directly, view it on GitHub https://github.com/rdp/os/pull/59#pullrequestreview-1104804059, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADBUGGJ4TBHL3FQM3ZRQLV56IH3ANCNFSM5VDWO3MQ . You are receiving this because you were mentioned.Message ID: @.***>

pboling commented 2 years ago

@rdp No, I'll do that upgrade separately.

pboling commented 1 year ago

@rdp I have fixed the issues with the test suite. In order to delay the upgrade to a newer RSpec I had to constrain the gem version dependencies in the gemspec to the same patch level as what had been used in the last "known good" state, which I assumed was in the Gemfile.lock.

This branch now works for both bundle install, and bundle exec rspec, and all tests pass. There were two bugs in the test suite that have been fixed.

rdp commented 1 year ago

OK is there a change in this PR? Or is it the following PR's? Github doesn't show it very clearly LOL.

pboling commented 1 year ago

I am doing individual PRs, and this PR is one of those. It is a complete, internally consistent change set, which passes all specs.

In addition I have a separate tracking branch, and associated PR, which is a collection of the sum total of the work I am doing to modernize the gem. It helps put all the changes in context together... and it will shrink in size as the individual PRs are merged.

Do you not use GitHub from a computer?

pboling commented 1 year ago

@rdp For some reason I do not have a merge option. Is a review required for a merge option to appear?

Update - I had to accept the invite. 🤦