Make LintRoller::Plugin sortable#7
Conversation
Typically, it is expected that `LintRoller::Plugin#about` includes a name, and sorting would be performed based on that name. However, currently, executing `sort` on LintRoller results in the following error: ```console ArgumentError: comparison of LintRoller::PluginTest::SampleRoller with LintRoller::PluginTest::AnotherRoller failed ``` This PR makes `LintRoller::Plugin` sortable by name. I considered including the version as well, but I could not come up with any use cases where multiple plugins with the same name would need to be processed simultaneously.
aecdd2e to
89e7bda
Compare
|
This looks fine to me. I also like the idea of enhancing to use version as secondary sort (not necessary to land in this PR, IMO) Would you mind sharing a bit more about the use case for this? I'm curious what scenarios want plugins to be sorted. Standard itself would likely not want plugins to be sorted for loading, because standard applies a "first-in wins" rule for configurations. Therefore, the order that plugins are listed and included must be preserved per the user's configuration. But that shouldn't stop this PR. |
|
Thanks for the response. You are right that I should have described the use case. Currently, the order of plugins shown by $ bundle exec rubocop -V
1.84.0 (using Parser 3.3.10.1, rubocop-ast 1.49.0, analyzing as Ruby 2.7, running on ruby 4.1.0) [arm64-darwin24]
- rubocop-rake 0.7.1
- rubocop-performance 1.26.1
- rubocop-rspec 3.9.0By contrast, with the approach used before the introduction of lint_roller, plugins were always displayed in sorted order. From a user's perspective, having the list of plugin names shown by |
|
|
||
| def <=>(other) | ||
| about.name <=> other.about.name | ||
| end |
There was a problem hiding this comment.
Should we include Comparable now that this implements the interface?
There was a problem hiding this comment.
The use case was simply to be able to call sort without raising an error, not to compare objects via comparison operator methods. For that reason, Comparable is not mixed-in, and there isn't a strong preference either way.
There was a problem hiding this comment.
Understood, thank you.
I like the consistency of being a Comparable when implementing the <=> interface, so I've added it to your branch. Merging momentarily.
Thank you for the contribution and your patience with this PR.
In order to be #sort able, plugins must be comparable via #<=>
Typically, it is expected that
LintRoller::Plugin#aboutincludes a name, and sorting would be performed based on that name. However, currently, executingsorton LintRoller results in the following error:ArgumentError: comparison of LintRoller::PluginTest::SampleRoller with LintRoller::PluginTest::AnotherRoller failedThis PR makes
LintRoller::Pluginsortable by name. I considered including the version as well, but I could not come up with any use cases where multiple plugins with the same name would need to be processed simultaneously.