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

Add new ruleset, gitignore, with rules required_patterns and forbidden_patterns #357

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

paulo-ferraz-oliveira
Copy link
Collaborator

@paulo-ferraz-oliveira paulo-ferraz-oliveira commented Sep 15, 2024

It'll use files identified by dirs+filter (which should be .gitignore) and apply
(at the moment, the only existing) rule gitignore_patterns with the default value found in the documentation.

Closes #316.

Considerations

  • rule name gitignore_patterns is up for discussion replace with two rules: required_patterns and forbidden_patterns
  • config. option all_of (name and content) is up for discussion the option name shall be regexes
  • ruleset name gitignore is up for discussion (though I'm rather fond of it)
  • implementation in elvis_project is up for discussion (the implementation itself, too, but I'm talking more about the placement of the code)
  • we may have to revisit the "since" once we discuss when to release stuff tentatively make it 4.0.0
  • the need for further tests is up for discussion
  • the error message is up for discussion

Copy link
Member

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

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

Can we make two rules, as follows?

  • required_patterns: for stuff that must be present in the file (i.e., the current rule)
  • forbidden_patterns: for stuff that must not be present in the file (including, for instance, rebar.lock)?

And maybe we can put them in their own module (elvis_gitignore or something like that), instead of elvis_project?

RULES.md Show resolved Hide resolved
@elbrujohalcon elbrujohalcon added this to the 4.0.0 milestone Sep 16, 2024
@paulo-ferraz-oliveira
Copy link
Collaborator Author

Can we make two rules, as follows?

Why not options?

@paulo-ferraz-oliveira
Copy link
Collaborator Author

And maybe we can put them in their own module (elvis_gitignore or something like that)

rebar_config and elvis_config are part of "project", which is what this rule wants to be. Why should we separate into different files?

@elbrujohalcon
Copy link
Member

Can we make two rules, as follows?

Why not options?

Mostly to have a ruleset with 2 rules, instead of one. 😜

@elbrujohalcon
Copy link
Member

And maybe we can put them in their own module (elvis_gitignore or something like that)

rebar_config and elvis_config are part of "project", which is what this rule wants to be. Why should we separate into different files?

Fair enough. Let it be in elvis_project. :)

@paulo-ferraz-oliveira
Copy link
Collaborator Author

Brujo I think you need to update the settings for this

image

@paulo-ferraz-oliveira
Copy link
Collaborator Author

If it's not much, would you check the stuff above (initial description) that needs to be updated (?) - or maybe the checks are blocked to the author?

What's missing?

It'll use files identified by `dirs`+`filter`
(which should be `.gitignore`) and apply
(at the moment, the only existing) rule
`gitignore_patterns` with the default value found in
the documentation

## Options

- `all_of :: [string()]`.
Copy link
Member

Choose a reason for hiding this comment

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

If we're going to go with a single rule instead of 2… For the options I prefer the names include and exclude. Even more, include_patterns and exclude_patterns.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW… I would still prefer to have 2 rules, one for what needs to be included and one for what needs to be excluded… so that we don't need to deal with the priorities of the application of the filters. In other words, if the user wants to put a rule to include "rebar*" and a rule to exclude "rebar.lock"… it's up to them to put a line for a rebar thing that's not rebar.lock.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, excludes makes sense: I was initially just thinking about my use case. Would the default list be empty?

Copy link
Member

@elbrujohalcon elbrujohalcon Sep 19, 2024

Choose a reason for hiding this comment

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

No, it should have rebar.lock. 😉
And maybe elvis.config 🤪

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm assuming rebar.lock is really something you want, and the other one was a joke 😄.

Copy link
Member

Choose a reason for hiding this comment

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

You're 100% right.

Copy link
Member

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

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

I think I still prefer to have 2 rules: one for required patters and one for excluded patterns. In that case, I would call the option just patterns or (to match other rules) regexes.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

We can go with two rules, sure. One's the mirror of the other one so duplicating code and tests + adjusting shouldn't take long.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

I've updated the description: I'm gonna make it two rules, each with the same option name. And I'm tentatively saying "since" is 4.0.0.

@paulo-ferraz-oliveira paulo-ferraz-oliveira changed the title Add new ruleset, gitignore, with rule gitignore_patterns Add new ruleset, gitignore, with rules required_patterns and forbidden_patterns Sep 18, 2024
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.

New ruleset gitignore
2 participants