Skip to content

Conversation

@marcciosilva
Copy link

Hi! Newbie to the project here, took a stab at trying to implement the enhancement from #4197.
Hope it makes sense to you to include the grouping code in the sorting processor (made sense to me since I wanted to preserve the eventual sorting within the linter 'group' - it could be done in a separate processor adding extra processing but keeping concerns apart, so I'm obviously open to suggestions!).

@boring-cyborg
Copy link

boring-cyborg bot commented Jan 18, 2024

Hey, thank you for opening your first Pull Request !

@CLAassistant
Copy link

CLAassistant commented Jan 18, 2024

CLA assistant check
All committers have signed the CLA.

@marcciosilva marcciosilva force-pushed the group-results-by-linter branch from 1c86631 to b34b77b Compare January 18, 2024 17:39
@ldez ldez self-requested a review January 18, 2024 20:26
@ldez ldez added the area: output Related to issue output label Jan 18, 2024
@ldez
Copy link
Member

ldez commented Jan 18, 2024

Hello,

I think an option only to "group" by linter name is too specific.

Otherwise, I don't understand the need for this kind of sorting, what is the usage of this?

@ldez ldez added the waiting for: contributor feedback Requires additional feedback label Jan 18, 2024
@marcciosilva
Copy link
Author

marcciosilva commented Jan 19, 2024

I was aiming to address the #4197 issue - I'm guessing this could be useful if e.g. you're solving misspellings, you might as well just fix all misspellings at once. Might be inefficient in terms of file traversing but it's the same type of work.

Same thing for other stuff like refactoring code to reduce cyclomatic complexity in functions. Might be annoying to 'context switch' between different types of work while fixing issues? That's what comes to mind.

But yeah it'd be perfectly reasonable to just close this (along with the issue) since it's too specific.

@ldez ldez added declined and removed waiting for: contributor feedback Requires additional feedback labels Jan 19, 2024
@ldez
Copy link
Member

ldez commented Jan 19, 2024

As we don't understand the usage and the option proposal is not enough "generic", we will close this PR, but the issue will stay open.

In any case, thank you for your implementation proposal.

@ldez ldez closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: output Related to issue output declined

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants