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

replaced asterisk string literal occurrences with constant #2344

Conversation

haiderashfaq
Copy link
Contributor

resolves #2343

Before contributing, please read our contributing guidelines and code of conduct.

Overview

Describe the changes present in the pull request

Submitter Checklist:

  • Include a link to the related GitHub issue, if applicable
  • Include a security review link, if applicable

Testing

The agent includes a suite of unit and functional tests which should be used to
verify your changes don't break existing functionality. These tests will run with
GitHub Actions when a pull request is made. More details on running the tests locally can be found
here for our unit tests,
and here for our functional tests.
For most contributions it is strongly recommended to add additional tests which
exercise your changes.

Reviewer Checklist

  • Perform code review
  • Add performance label
  • Perform appropriate level of performance testing
  • Confirm all checks passed
  • Add version label prior to acceptance

@github-actions github-actions bot added the community To tag external issues and PRs submitted by the community label Dec 2, 2023
@fallwith
Copy link
Contributor

fallwith commented Dec 3, 2023

Hi @haiderashfaq,

Thanks very much for your PR! These changes will help keep object allocation and garbage collection sweeps down to improve performance and are very much appreciated.

It looks like the change to lib/new_relic/agent/instrumentation/sinatra/ignorer.rb caused some unit test failures.

Would you please the following change?

- routes = [ASTERISK] if routes.empty?
+ routes = [::NewRelic::ASTERISK] if routes.empty?

That should fix it. None of the other files changed caused any test failures, so all of those other changes are good as-is.

@fallwith
Copy link
Contributor

fallwith commented Dec 4, 2023

Thank you for your contribution, @haiderashfaq! This change will go out with the next agent release.

@fallwith fallwith merged commit 713200b into newrelic:dev Dec 4, 2023
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community To tag external issues and PRs submitted by the community
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Replace literal single asterisk string occurrences with the ASTERISK constant
2 participants