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

feat: add ability to specify branches for (any)-file-content rules #293

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

Conversation

Brend-Smits
Copy link
Collaborator

Motivation

Some rules require you to check not just the default branch, but also other branches.

Proposed Changes

This is now possible by using the 'branches' option. Currently only works for the any-file-contents and file-contents rule.
The skipDefaultBranch option can be used if the user wants to skip the default branch from being checked. This will also fix #15

Test Plan

Run NPM Tests. Example rulesets can be found here: https://github.com/Brend-Smits/repolinter-tests/tree/main/rulesets
We should consider moving this repository to the todogroup GitHub organization.

Note: This pull request should be merged after #290. The rulesets that we use with testing, also refer to this rule. This is also the reason why some tests will fail. This will automatically be resolved when the other PR is merged.

Signed-off-by: Brend Smits [email protected]

@Brend-Smits Brend-Smits requested a review from hyandell as a code owner March 31, 2023 13:22
@neilzhao-aws
Copy link

Thinking on the feature of "run repolinter on a specific branch instead of default", I'd approach it by enhance the -g git cli option by allow "repo:branch" on top of just "repo", need to look into how to achieve that with simple-git.
That's allow enable the repo linter to:

  1. Run any branch/commit/tag following the general repo:branch, repo:commit convention.
  2. Run on all ruleset instead of limited subset of rulesets.
  3. Make the cost (time for running repolinter) clear to the user, customer would need to curate a list of branch and run in a script, can do easy math to get the total run time.

@Brend-Smits
Copy link
Collaborator Author

Brend-Smits commented Apr 1, 2023

Thinking on the feature of "run repolinter on a specific branch instead of default", I'd approach it by enhance the -g git cli option by allow "repo:branch" on top of just "repo", need to look into how to achieve that with simple-git.
That's allow enable the repo linter to:

Thanks for commenting!

I see where you are coming from, the problem with that approach is that you can't configure this per rule. You will lose the ability to run rule X for just the default branch, while running rule Y for various branches (all within the same ruleset).
For many rules, it does not make sense to check multiple branches.

@zhaoyuheng200
Copy link
Member

I see. In that case, may I request to move the feature change away from file-contents and setup its own rule?
Reason being:

  1. All current rules are working with traditional file/dir, this would be the first rule that directly working git, I think it's necesasry to call that out and make it a separate rule, maybe git-file-contents. The new rule can invoke file-contents.
  2. The test cases for this feature would be cleaner, limited to testing sequential git/file-contents.

@hyandell
Copy link
Member

hyandell commented Apr 6, 2023

My couple of thoughts here are:

  • I don't understand what 'sequentialOnly' is for. As a parameter it sounds like it is telling something not to run in multiple threads; but CLI-UI wise, it's not clear what that something is. It doesn't seem introduced by feat: add any-file-contents rule #290.
  • +1 for exploring as a separate rule.

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.

For some options, verify all versions of the repository
4 participants