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

DD templating: conditionally invoke requires #2917

Merged
merged 2 commits into from
Oct 21, 2024
Merged

DD templating: conditionally invoke requires #2917

merged 2 commits into from
Oct 21, 2024

Conversation

fallwith
Copy link
Contributor

as per #2844, don't perform require_relative on content that won't be used unless all dependency detection checks succeed

as per #2844, don't perform `require_relative` on content that won't be
used unless all dependency detection checks succeed
Comment on lines 17 to 20
require_relative '<%= @snake_name.downcase %>/instrumentation'
require_relative '<%= @snake_name.downcase %>/chain'
require_relative '<%= @snake_name.downcase %>/prepend'

Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on going a step further... to require the instrumentation file here, and the chain and/or prepend files within the condition below?

require_relative '<%= @snake_name.downcase %>/instrumentation'

  if use_prepend? 
    require_relative '<%= @snake_name.downcase %>/prepend'
    prepend_instrument <%= @class_name %>, NewRelic::Agent::Instrumentation::<%= @class_name %>::Prepend
 else
   require_relative '<%= @snake_name.downcase %>/chain'
   chain_instrument NewRelic::Agent::Instrumentation::<%= @class_name %>::Chain
 end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, addressed with 1c6d8a3.

when chaining or prepending, only bring in the appropriate chain or
prepend code
Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link

SimpleCov Report

Coverage Threshold
Line 93.78% 93%
Branch 69.71% 50%

@fallwith fallwith merged commit 9bcd02e into dev Oct 21, 2024
34 checks passed
@fallwith fallwith deleted the it_(all)_depends branch October 21, 2024 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Code Complete/Done
Development

Successfully merging this pull request may close these issues.

2 participants