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

Updates so things work with frozen string literals #567

Merged
merged 2 commits into from
Aug 16, 2017

Conversation

pat
Copy link
Contributor

@pat pat commented Jul 2, 2017

These changes get Thor working when frozen string literals are enabled.

Regarding test suite dependencies and their compatibility with frozen string literals:

  • RSpec is compatible in their latest git commits (for rspec-core, rspec-expectations, etc).
  • simplecov and its dependency simplecov-html are compatible in their latest git commits.
  • Addressable is compatible in this PR: Get tests passing with frozen string literals. sporkmonger/addressable#260
  • Webmock is not yet compatible - I'm working on it, though I've also uncovered a frozen string literal bug in stdlib. The patches I've made locally (not yet submitted to a PR) are enough to get Thor's build green though.

@pat
Copy link
Contributor Author

pat commented Jul 2, 2017

With regards to the stdlib bug (net/protocol) - looks like it's fixed in trunk, though has not (yet) been backported into the next 2.4.x release.

I'm happy to add the pragma comment (in this PR or a new one) to at least ensure regressions aren't introduced, if you'd like?

@segiddins
Copy link
Contributor

Maybe just add the magic comment to the Thor source files?

@pat
Copy link
Contributor Author

pat commented Jul 3, 2017

@segiddins: I'm certainly happy to make that change if it's the maintainers' preference, just didn't wish to presume, because it involves editing pretty much every file, and then needs to be remembered for any files that get added to the project further down the track.

@@ -340,7 +340,8 @@ def capture(*args)
with_output_buffer { yield(*args) }
end

def with_output_buffer(buf = "") #:nodoc:
def with_output_buffer(buf = "".dup) #:nodoc:
raise "No: #{buf}" if buf.frozen?
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a better descriptive error here. Also I think the exception should be an ArgumentError

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 @rafaelfranca - that was actually a debugging line that I shouldn't have committed in the first place. Have kept the raise, using ArgumentError, and put in a more useful error message.

Previous line was actually debugging that shouldn't have been committed. This is much better.
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.

3 participants