Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

AttributeTranslator optimization #1944

Merged
merged 3 commits into from
Dec 15, 2019
Merged

AttributeTranslator optimization #1944

merged 3 commits into from
Dec 15, 2019

Conversation

ericproulx
Copy link
Contributor

Still memory profiling

Allocated String Report
-----------------------------------
      7640  "d"
      3820  /Users/eproulx/.rbenv/versions/2.4.9/lib/ruby/gems/2.4.0/gems/grape-1.2.6/lib/grape/router/attribute_translator.rb:16
      3820  /Users/eproulx/.rbenv/versions/2.4.9/lib/ruby/gems/2.4.0/gems/grape-1.2.6/lib/grape/router/attribute_translator.rb:18

      1248  "s"
       624  /Users/eproulx/.rbenv/versions/2.4.9/lib/ruby/gems/2.4.0/gems/grape-1.2.6/lib/grape/router/attribute_translator.rb:16
       624  /Users/eproulx/.rbenv/versions/2.4.9/lib/ruby/gems/2.4.0/gems/grape-1.2.6/lib/grape/router/attribute_translator.rb:18

       936  "p"
       468  /Users/eproulx/.rbenv/versions/2.4.9/lib/ruby/gems/2.4.0/gems/grape-1.2.6/lib/grape/router/attribute_translator.rb:16
       468  /Users/eproulx/.rbenv/versions/2.4.9/lib/ruby/gems/2.4.0/gems/grape-1.2.6/lib/grape/router/attribute_translator.rb:18

d = request_method
s = :requirements
p = regexp

Instead of using method_missing to fetch the data, I thought it would be better to define the methods

@ericproulx
Added request_method and requirements has methods instead using metho…
153051a 
…d_missing since it's defined by defaults.

Added regexp and index in Any class instead of falling back on method_missing
@ericproulx
Change changelog number
81978af
@grape-bot
Copy link

grape-bot commented Dec 14, 2019 •
edited
Loading

1 Warning
⚠️ There’re library changes, but not tests. That’s OK as long as you’re refactoring existing code.

Generated by 🚫 danger

@dblock
Copy link
Member

dblock commented Dec 15, 2019

Good stuff. See my comment above.

@dblock dblock merged commit afcca35 into ruby-grape:master Dec 15, 2019
@dblock
Copy link
Member

dblock commented Dec 15, 2019

👍 merged

basjanssen pushed a commit to basjanssen/grape that referenced this pull request Feb 28, 2020
@ericproulx @basjanssen
AttributeTranslator optimization (ruby-grape#1944)
e9d4973 
Added request_method and requirements has methods instead using method_missing since it's defined by defaults. Added regexp and index in Any class instead of falling back on method_missing
@ericproulx ericproulx mentioned this pull request Dec 21, 2023
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@dblock dblock dblock left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ericproulx @grape-bot @dblock