-
Notifications
You must be signed in to change notification settings - Fork 10
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
Make nsq-cluster work on ruby 2.7-3.2 / arm64 #19
Conversation
f513791
to
5ddd9b4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No concerns, just a few questions.
spec/spec_helper.rb
Outdated
@@ -19,5 +20,5 @@ | |||
end | |||
|
|||
RSpec.configuration.before :each do | |||
FakeWeb.allow_net_connect = %r[^https?://#{Regexp.escape('127.0.0.1')}.*] | |||
#FakeWeb.allow_net_connect = %r[^https?://#{Regexp.escape('127.0.0.1')}.*] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just remove this block instead of commenting?
spec/spec_helper.rb
Outdated
@@ -10,7 +11,7 @@ | |||
|
|||
require 'nsq-cluster' | |||
|
|||
require 'fakeweb' | |||
#require 'fakeweb' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove dead code?
.github/workflows/run-tests.yml
Outdated
shell: bash | ||
run: | | ||
set -euo pipefail | ||
which nsqd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to be suspicious of the prior step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - there was some debugging I had to do - I think the tests that ran in the subsequent step weren't accepting the modified PATH (vs installing NSQ binaries into the /usr/local/bin path) in parts of my debugging - I think I can get rid of the step
15bf525
to
3d91ce7
Compare
This PR contains changes to make nsq-cluster work on newer Ruby versions / arm machines (or at least make the specs fully runnable on these versions).