Skip to content

Tracking issue: configuration for linters and further CI checks #30

@grunweg

Description

@grunweg

Currently, the project does not set up any configuration for running linters etc. I think it should do so, depending what the goal of the project is. Let's go through the various checks in order:

  • I think mk_all should always be enforced. (If a user opts out of that by using globs, they can delete this bit also.) feat: add mk_all step to CI #29 does that

  • environment linters: it may depend on the linter; blocked on external PRs

    • some of them are almost surely useful (such as the defLemma linter)
    • the unusedHave linter is probably also useful, perhaps after fix: linter should have access to all messages, really  leanprover/lean4#8117 is backported to the current lean version
    • running the docBlame linter depends on the project's goal: if everything should be upstreamed to mathlib (as for the FLT project), it should absolutely be enabled. If the project is not meant to touch mathlib at all, it need not be. For projects with a mixed structure, enabling it only for the ToMathlib directory would be the most convenient. Such a setting cannot be configured yet, though... Thus, I would suggest asking the user about this in customize_template.py
  • mathlib's syntax linters: "everything" be a possible option; feat: implement "linter sets" that can be turned on as a group leanprover/lean4#8106 will help with that.

    • the header linter (which enforces copyright headers) could be up for discussion. Many downstream projects don't enable it (but add copyright whenever significant new material is upstreamed to mathlib). Adding a copyright header can be a barrier to entry for new contributors, so this may be a sensible choice.
    • I would advocate for the flexible linter to be enabled by default: non-terminal simps are a significant source of churn which new contributors often learn about the hard way. Mathlib has not enabled that linter mostly because there are ~200 errors which are harder to fix. Most new projects should be fine enabling it. (If they know and care about this, they can disable it again.)
    • the style linters are... well, questions of style (hence subjective)
    • the file length linter is a matter of taste: perhaps don't enable it by default
  • running shake: I think doing so by default is sensible
    (A noshake.json file is sensible; a nolints.json file only if needed. For new projects, there is no code yet, hence no need for this.)

  • mathlib's text-based linters: these used to be very useful; that that many checks have been (or are being) rewritten as syntax linters, this is less clear-cut. This depends on feat(lint-style): enable running on downstream projects leanprover-community/mathlib4#21476, but otherwise I'd tend to enable it by default as it's quick and can be useful.

  • You may also want to lint for "import Mathlib" statements, as these are often very broad.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions