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

Find More Rules! #281

Closed
elbrujohalcon opened this issue Feb 28, 2023 · 10 comments
Closed

Find More Rules! #281

elbrujohalcon opened this issue Feb 28, 2023 · 10 comments
Assignees
Milestone

Comments

@elbrujohalcon
Copy link
Member

Let's check other linters, find their list of rules/guidelines and copy use them for inspiration 🙄

@elbrujohalcon elbrujohalcon added this to the 3.0.0 milestone Feb 28, 2023
@elbrujohalcon elbrujohalcon self-assigned this Feb 28, 2023
@elbrujohalcon
Copy link
Member Author

@paulo-ferraz-oliveira
Copy link
Collaborator

Yeah, since I started using Credo I got inspired by some of its rules. I'll try to list them below so we can discuss further.

@elbrujohalcon
Copy link
Member Author

Feel free to do it, but keep in mind that I'm currently (during working hours) going over that list myself and adding the ones that I find useful/interesting as tickets in the 3.0.0 milestone.

@paulo-ferraz-oliveira
Copy link
Collaborator

Credo.Check.Consistency.LineEndings
Credo.Check.Warning.UnsafeToAtom
Credo.Check.Warning.LeakyEnvironment
Credo.Check.Warning.BoolOperationOnSameValues
Credo.Check.Refactor.Apply
Credo.Check.Readability.TrailingBlankLine
Credo.Check.Readability.StrictModuleLayout
Credo.Check.Readability.RedundantBlankLines
Credo.Check.Readability.PreferUnquotedAtoms
Credo.Check.Readability.LargeNumbers

@paulo-ferraz-oliveira
Copy link
Collaborator

I've recently (for work too) gone over that list for all rules. I'm convinced that (from Credo) Elvis shouldn't be missing much more than the ones listed above.

@elbrujohalcon
Copy link
Member Author

I'll review your list as well tomorrow.

@elbrujohalcon
Copy link
Member Author

Let's go through your list, @paulo-ferraz-oliveira

  • Credo.Check.Consistency.LineEndings: While I have no problem adding such a rule, I would argue that this is a formatter issue. It won't be a priority for me.
  • Credo.Check.Warning.UnsafeToAtom: This can be achieved already using a clever configuration for No Call. Besides, I think this rule fits more nicely in a tool like PEST than Elvis.
  • Credo.Check.Warning.LeakyEnvironment: This one seems interesting, but I'm not sure how it's implemented / what's the algorithm for it. Also, I think this one fits better in PEST than here.
  • Credo.Check.Warning.BoolOperationOnSameValues: This is a great one! I'll write a ticket for it.
  • Credo.Check.Refactor.Apply: This can be achieved already using a clever configuration for No Call.
  • Credo.Check.Readability.TrailingBlankLine: While I have no problem adding such a rule, I would argue that this is a formatter issue. It won't be a priority for me.
  • Credo.Check.Readability.StrictModuleLayout: A really nice rule. Not trivial to implement because it has to be as configurable as the one in Credo, but I'll write a ticket for it nonetheless.
  • Credo.Check.Readability.RedundantBlankLines: While I have no problem adding such a rule, I would argue that this is a formatter issue. It won't be a priority for me.
  • Credo.Check.Readability.PreferUnquotedAtoms: While I have no problem adding such a rule, I would argue that this is a formatter issue. It won't be a priority for me.
  • Credo.Check.Readability.LargeNumbers: We already have Numeric Format. I think we've got this one covered.

@elbrujohalcon
Copy link
Member Author

elbrujohalcon commented Mar 1, 2023

I'm still going through the whole list of rules… I'll pick the ones I would like to have in Elvis and turn them into tickets, including Credo.Check.Warning.BoolOperationOnSameValues, and Credo.Check.Readability.StrictModuleLayout

@paulo-ferraz-oliveira
Copy link
Collaborator

For the ones that are "formatter" issues we could still have the issues created for elvis and they might be picked up; we already have some (implemented and released) that are "formatter"-based, anyway :)

As for LargeNumbers, yeah, I forgot to remove it when doing the second review on my list.

Agree for everything else.

@elbrujohalcon
Copy link
Member Author

Yeah, @paulo-ferraz-oliveira ! Do create the issues that are formatter-related, of course! Just don't put them on the 3.0.0 milestone ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants