MetaMask / module-lint

Analyzes one or more repos for divergence from a template repo.
1 stars 3 forks source link

Add checks for general module template adherence #6

Open desi opened 10 months ago

desi commented 10 months ago

Although we have checks for specific tools — ESLint, TypeScript, etc. — it may be that we make a change to the module template and don't have a specific rule around it in module-lint. Therefore, we need a set of rules to act as catch-alls. Although these rules may not be perfect — the output of these rules may not be useful as the output of more specific rules and may not account for all of the intentional divergences from the module template — it's better than nothing.

Given this, for a given project, we want to ensure that:

Gudahtt commented 6 months ago

Suggestions:

mcmire commented 6 months ago

A question was raised in standup this morning about the overlap between the rule being discussed in this ticket and the existing rules we've completed and have planned. If there are already certain files we are checking via other rules, it seems that this rule would also check those files, so should we exclude them from this rule?

We could do this, but we would have to make two assumptions:

  1. We would have to assume that all rules are already using the logic presented in this ticket, i.e., they use recursive containment to check configuration instead of equality.
  2. We would also have to assume that all rules that operate on a file check the full content of the file and not a part of it.

The first assumption is valid; the existing rules do not make a containment check, but we can easily fix that. The second assumption, however, creates a problem. One of the guiding principles behind the existing rules is that they should be as bite-size and have as few responsibilities as possible. This way, a project can disable and enable individual rules to suit its needs. However, in taking this fine-tuned approach we cannot guarantee that all of the files and configuration in the module template are being covered by a rule. In other words, if a change is made to the module template in the future, it is possible there may not be a rule in module-lint which verifies that change. Therefore, we need a way to address any gaps in coverage that may exist at any given time. Hence, the rule outlined in this ticket would serve that purpose.

mcmire commented 6 months ago

I am going to suggest, based on the size of this ticket, that we hold off until we get through all of the other ones first. If the main goal of this ticket is to address potential gaps in existing rules, perhaps there is a way we can compare the rules we have with the module template and verify that everything is covered. Or perhaps having gaps is just an inevitability based on the way this tool is designed. I'll have to give this a bit more thought.