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

Prevent duplicate ids #221

Merged
2 commits merged into from
Jun 5, 2019
Merged

Prevent duplicate ids #221

2 commits merged into from
Jun 5, 2019

Conversation

jesseadams
Copy link

Closes #220

existing_def
@duplicate_ids << {
id: id,
new_message: message,
Copy link

@ghost ghost Jun 4, 2019

Choose a reason for hiding this comment

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

the new v. old is fine, but poking at two things:

  1. is the order stable for this to have meaning?
  2. on the off chance we have > 2 collisions... i guess it shows A/B and then B/C?

Perhaps make @duplicate_ids a Hash with id key and array of messages?

I guess there's no way to say where the duplicate class is coming from? for core rules this is a bug that we rightfully stop the release pipeline for..... but when there are custom rule directories that users are pointing to....

Copy link
Author

Choose a reason for hiding this comment

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

I have suggested that custom rules have their own number system with a prefix of C. But yeah, this is just a barebones approach to fail CI builds when there are duplicates. The messages aren't perfect but they enabled me to quickly troubleshoot the one duplicate we had. If we want something fancier, we can always iterate on it in the future as need arises.

@ghost ghost merged commit d17e8bf into master Jun 5, 2019
@ghost ghost deleted the prevent-duplicate-ids branch January 6, 2020 21:58
This pull request was closed.
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.

Prevent duplicate rule IDs from occurring
1 participant