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

Report unused ignore lines in config #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hlascelles
Copy link

@hlascelles hlascelles commented Feb 19, 2021

This change allows traceroute to report when there are unused lines in its config. ie, lines that never "match to ignore" anything.

Unused lines are not just a buildup of historic cruft, they could also unintentionally ignore matching code further down the line if the regex is left in, and then it matches something else entirely.

It will currently fire all the time - up to you if you want to hide it behind an ENV.

This change allows traceroute to report when there are unused lines in its config. ie, lines that never "match to ignore" anything.

Unused lines are not just a buildup of historic cruft, they could also unintentionally ignore matching code further down the line if the regex is left in, and then it matches something else entirely.
@hlascelles
Copy link
Author

@amatsuda What do you make of this change?

@compwron
Copy link

This feels like it would be useful to my team

@hlascelles
Copy link
Author

@amatsuda Would you like to include this change?

@hlascelles
Copy link
Author

@amatsuda what do you think?

@hlascelles
Copy link
Author

@amatsuda Any thoughts?

@hlascelles
Copy link
Author

@MaximilianoGarciaRoe do you have the permissions to merge this and cut a new gem? I can make changes if you like.

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