🚑️ Remove circular dependency, fixes #55, #60 #59
Conversation
|
@rdp Ping! |
| @@ -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<os>.freeze, [">= 0"]) | |||
There was a problem hiding this comment.
The os gem can't depend on itself. I think this is part of the messy legacy of jeweler.
There was a problem hiding this comment.
Yeah I think jeweler was auto inserting this, scary...
|
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 <#59 (comment)>:
> @@ -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<os>.freeze, [">= 0"])
The os gem can't depend on itself. I think this is part of the messy
legacy of jeweler.
------------------------------
In Gemfile.lock <#59 (comment)>:
> 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
<#59 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAADBUGGJ4TBHL3FQM3ZRQLV56IH3ANCNFSM5VDWO3MQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@rdp No, I'll do that upgrade separately. |
pboling
force-pushed
the
bug/55-bundle-install
branch
from
15ccbae to
9bb625c
Compare
pboling
changed the title
|
@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 This branch now works for both |
pboling
force-pushed
the
bug/55-bundle-install
branch
from
9bb625c to
66bc609
Compare
|
|
||
| if OS.mac? | ||
| ENV.stub(:[]).with('HOME').and_return('/home/user') | ||
| assert OS.app_config_path('appname') == '/home/xdg/Library/Application Support/appname' |
There was a problem hiding this comment.
It's clear you weren't running this on a Mac 😄
| @@ -121,7 +121,8 @@ | |||
| it "has working cpu count method" do | |||
| cpu_count = OS.cpu_count | |||
| assert cpu_count >= 1 | |||
| assert (cpu_count & (cpu_count - 1)) == 0 # CPU count is normally a power of 2 | |||
There was a problem hiding this comment.
I use a 6-core, 12 vCPU Macbook Pro. The power of 2 idea is no longer reasonable, so I have added "or an even number" as a secondary condition.
|
OK is there a change in this PR? Or is it the following PR's? Github doesn't show it very clearly LOL. |
|
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? |
|
@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. 🤦 |
Fixes #55 & #60
Signed-off-by: Peter Boling peter.boling@gmail.com