-
Notifications
You must be signed in to change notification settings - Fork 19
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
Remove debug output #21
base: master
Are you sure you want to change the base?
Conversation
01cf75b
to
eef2917
Compare
fee10c7
to
7eb5078
Compare
7eb5078
to
827ac62
Compare
…[]).inspect` in a controller Stacktrace: /Users/nicolas/PROJECTS/CONCERTO/concerto/.bundle/ruby/3.3.0/bundler/gems/trailblazer-endpoint-827ac62bdb4c/lib/trailblazer/endpoint/options.rb:80:in `block in CallDirective': no implicit conversion of nil into Hash (TypeError) config = callable.(ctx, **ctx) # e.g. ApplicationController.options_for_endpoint
@@ -77,6 +77,7 @@ def self.add(normalizer, directive_name, options) | |||
|
|||
def self.CallDirective(callable, option_name) | |||
->((ctx, flow_options), *) { | |||
ctx ||= {} |
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.
Using ||=
usually implies you have lost track of how your data flows, it's always an anti-pattern for me! In what cases is that required for you, if I may ask?
Also, off-topic, @n-rodriguez may I ask you some questions on https://trailblazer.zulipchat.com of how you use endpoint
? Background is, I have another branch where I threw out a lot of complex logic of this gem and changed the runtime API and I'd like to hear a user's thoughts (and so far, you're the only user I know hahaha). ☕
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.
In what cases is that required for you, if I may ask?
For debugging :)
When I run this code without the patch (in config/puma.rb
) :
before_fork do
PumaSettings.info '* Finalizing master process'
#puts CampingsController.instance_variable_get('@normalizers')[:endpoints].call([]).inspect
puts ConcertoFrance::PagesController.instance_variable_get('@normalizers')[:endpoints].call([]).inspect
3.times { GC.start }
GC.compact
#puts CampingsController.instance_variable_get('@normalizers')[:endpoints].call([]).inspect
puts ConcertoFrance::PagesController.instance_variable_get('@normalizers')[:endpoints].call([]).inspect
end
it fails with this error :
/Users/nicolas/PROJECTS/CONCERTO/concerto/.bundle/ruby/3.3.0/bundler/gems/trailblazer-endpoint-827ac62bdb4c/lib/trailblazer/endpoint/options.rb:80:in `block in CallDirective': no implicit conversion of nil into Hash (TypeError)
config = callable.(ctx, **ctx) # e.g. ApplicationController.options_for_endpoint
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.
With this patch we can see the Ruby GC bug : trailblazer/trailblazer-activity#60 (comment)
Reopen #20