Skip to content

[CHORE] Duplicate definition of _header #70

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

Merged
merged 1 commit into from
Jan 12, 2019

Conversation

maschwenk
Copy link
Contributor

I'm unsure what this code was originally trying to work around but I think this change makes more sense and is functionally the same.

Perhaps at some point calculating headers was expensive?

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

Thanks for your report.
Could you check my comment?

@@ -1243,9 +1243,11 @@ def inspect
str << " " << attr_name << ":" << a.inspect
end
end
_headers = headers
_headers = headers
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to just remove this line.
I don't want to call the same method twice.

@maschwenk maschwenk force-pushed the duplicate-headrs-definition branch from a847cb0 to 42893fa Compare January 12, 2019 07:47
@maschwenk
Copy link
Contributor Author

maschwenk commented Jan 12, 2019

@kou I've addressed your recommendations in 42893fa.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

Thanks!
I'll merge this.

@kou kou merged commit 6a63665 into ruby:master Jan 12, 2019
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.

2 participants