Decouple linting, formatting, and pre-commit from external dotfiles #7447

mark-pitblado · 2025-09-23T22:57:07Z

This PR removes the link between Max Platiiuk's dotfiles and the Specify repository for formatting, linting and the pre-commit config. I have tried to stay as close to a copy and paste operation as possible, however the following two changes were made:

Previously, flake was pulling from a repository on GitLab, which required a login when I was testing with pre-commit. Since developers and testers for Specify are developing on GitHub, it seemed to make sense to switch to a GitHub based repo so that no additional setup is required.
The markdownlint used in pre-commit required ruby during my testing. Since ruby isn't one of the languages used by the Specify project, it seemed prudent to swap this out for a markdownlint based on nodejs. Markdownlinters should be pretty similar, there are very few rules for markdown.

Eslint is definitely where things get the most complicated, particularly because I don't think eslint has been a part of the ci pipeline for a while due to the high number of errors it produces?

specify7/.github/workflows/test.yml

Lines 270 to 274 in cbff898

    
                   run: | 
        
                     # TODO: Once most errors are fixed remove "set +e" to not ignore errors 
        
                     set +e 
        
                     npx eslint --fix --color `echo "${{steps.changed.outputs.changed}}" | tr " " "\n"` 
        
                     set -e

I think it may be prudent to think about where in the pipeline each type of error is expected to be caught, and which tools are mandatory to setup. For example, if a developer has pre-commit setup, are they good to go? Or should they also run npm run test on their machine? Which jobs are handled by the ci runners, compared to local testing? A good candidate for developer documentation.

Note

Gitguardian has a pre-commit config that can catch secrets before they are commited as a pre-commit check (or pushed as a pre-push check), whereas I believe the PR integration will only catch the secret after it has been committed (it can be then rotated/revoked, but that's a pain). An example of a case where it matters where a check is run.

Dependency changes

The following dependencies were added to the Specify frontend. These were always dependencies for the project, they were just further upstream because they were dependencies for the dotfiles.

Checklist

Self-review the PR after opening it to make sure the changes look good and
self-explanatory (or properly documented)

Testing instructions

These changes are meta-changes, and shouldn't change any functionality of application code. The only test to be done is to run the ci and pre-commit pipelines and observe that there are no big changes suggested for files that have already been through the pipeline. Note that running the existing pre-commit configuration over the entire codebase will result in a lot of changes, it appears like many directories have never been touched by pre-commit (perhaps because they haven't seen changes in a while, or pre-commit is not consistently used).

Previously the code removed prettier-plugin-tailwindcss from base, however, now that all settings are defined directly, can just remove the plugin from the list.

This can be kept because it is like any other dependency in the pre-commit config, however, it should have a description since the title of the repository is ambiguous.

With gitlab, the user has to login to gitlab in order to use the hook. Since development of specify occurs on github, it makes sense to use the github version to prevent developers from having to setup another account.

markdownlint relies on a ruby gem, which requires a version of ruby that macOS machines don't ship with by default. Since ruby isn't a language that is in the specify codebase, it would be better to use a markdown linter that uses node.

Copies the rules previously in the import. There is no need for stylelint-config-prettier after stylelint version 15, see https://www.npmjs.com/package/stylelint-config-prettier

No longer necessary since these configuration variables are now directly in the file

Some debugging remains for how testFiles and abbreviationsConfig are handled

Tested and eslint now runs fine without errors

These are no longer necessary as dev dependencies

These were requirements in max's dotfiles, and so are now required by specify

Changes look good, single quote is respected, which is the biggest difference between specify's settings and the default. Ran over the entirety of js_src and no changes to 99% of files.

The ci pipeline requires the lockfile and package.json to be in sync, this should resolve that error.

Triggered by 28d7155 on branch refs/heads/issue-7402

mark-pitblado · 2025-09-24T17:24:10Z

04cc77c installs the latest versions of prettier and the plugins, don't see a reason not to use the latest version, as these are just formatters.

eslint-plugin-markdown is depracated, npm suggests eslint/markdown as a replacement

Triggered by c030cba on branch refs/heads/issue-7402

Since eslint is largely ignored in the current ci pipeline, starting from the defaults and building up may make sense.

… issue-7402

grantfitzsimmons

Thank you so much for doing this! @CarolineDenis, we should discuss what changes need to be made to address

Note that running the existing pre-commit configuration over the entire codebase will result in a lot of changes, it appears like many directories have never been touched by pre-commit (perhaps because they haven't seen changes in a while, or pre-commit is not consistently used).

mark-pitblado · 2025-10-08T18:14:22Z

This pull request isn't ready to merge just yet, mostly because I'm trying to work out a proposal for what to do with eslint. The current config is using a version of eslint (version 8) that is depracated. However, just switching back to the recommended configuration¹ will litter the codebase with comment strings that say to ignore a previous rule. In a perfect world I could get all the plugins and rules to line up.

As far as I can tell, the following are done at each stage of the pipeline:

Stage	Testing
Pre-commit	Runs frontend jest tests, regex testing, python typechecking, python linting, python formatting (black), prettier formatting, doctstring linting, markdown linting, docker compose linting.
On PR	GitGuardian (secret checking)
CI Workers	Eslint, prettier formatting, frontend tests, backend tests

Currently, the only thing that eslint will do is autofix what it can (which isn't supported for all rules, just some). Errors are ignored for both prettier and eslint in the pipeline, presumably because they were too restrictive at some time. Warnings are... well just warnings.

# TODO: Once most errors are fixed remove "set +e" to not ignore errors

Example: https://github.com/specify/specify7/actions/runs/18353184533/job/52278324256

I don't think stylelint is used anywhere, but there is a config for it, I guess if an individual developer wanted to set it up in their IDE they could use it.

Since the current ruleset is being ignored, I think it would be better to pull back to just the recommended config and see if that is something that should be adhered too, but that's just my two cents. Using the recommended would reduce dependencies by eliminating the need for all the plugins. ↩

mark-pitblado added 9 commits September 10, 2025 13:18

refactor(prettier): remove dotfiles import, copy over settings

0e6f229

Previously the code removed prettier-plugin-tailwindcss from base, however, now that all settings are defined directly, can just remove the plugin from the list.

docs(pre-commit): add description of what maxpatiiuk pre-commit does

bc011e8

This can be kept because it is like any other dependency in the pre-commit config, however, it should have a description since the title of the repository is ambiguous.

refactor(pre-commit): flake8 gitlab -> github

c5f646a

With gitlab, the user has to login to gitlab in order to use the hook. Since development of specify occurs on github, it makes sense to use the github version to prevent developers from having to setup another account.

chore(pre-commit): format file

b4b88f8

refactor(stylelint): copy rules from maxxxxxxdlp config

386589c

Copies the rules previously in the import. There is no need for stylelint-config-prettier after stylelint version 15, see https://www.npmjs.com/package/stylelint-config-prettier

chore(packages): remove maxpatiiuk prettier config

93816e5

No longer necessary since these configuration variables are now directly in the file

chore(eslint): move imported configuration into constants

3cd841a

Some debugging remains for how testFiles and abbreviationsConfig are handled

fix(eslint): access array from eslintConfig

23cac8a

Tested and eslint now runs fine without errors

github-project-automation bot added this to General Tester Board Sep 23, 2025

github-project-automation bot moved this to 📋Back Log in General Tester Board Sep 23, 2025

mark-pitblado changed the title ~~Issue 7402~~ Decouple linting, formatting, and pre-commit from external dotfiles Sep 23, 2025

mark-pitblado added 6 commits September 24, 2025 09:52

chore(packages): remove maxxxxxdlp dotfile packages

1239e7f

These are no longer necessary as dev dependencies

chore(packages): add prettier plugins for package and sh

04cc77c

These were requirements in max's dotfiles, and so are now required by specify

format(prettier): run prettier with specify config

cf8d2da

Changes look good, single quote is respected, which is the biggest difference between specify's settings and the default. Ran over the entirety of js_src and no changes to 99% of files.

ci(npm): push new lock file so that npm can complete

8fb9570

The ci pipeline requires the lockfile and package.json to be in sync, this should resolve that error.

Merge branch 'main' into issue-7402

28d7155

Lint code with ESLint and Prettier

0eab890

Triggered by 28d7155 on branch refs/heads/issue-7402

mark-pitblado added 6 commits September 24, 2025 11:06

chore(packages): add eslint dependencies from max's dotfiles

4897d65

chore(packages): replace eslint-plugin-markdown with eslint/markdown

34d5cf6

eslint-plugin-markdown is depracated, npm suggests eslint/markdown as a replacement

Merge branch 'main' into issue-7402

c030cba

Lint code with ESLint and Prettier

26274ae

Triggered by c030cba on branch refs/heads/issue-7402

refactor(eslint): switch to defaults

82ad88e

Since eslint is largely ignored in the current ci pipeline, starting from the defaults and building up may make sense.

Merge branch 'issue-7402' of https://github.com/specify/specify7 into…

8375f50

… issue-7402

grantfitzsimmons added this to the 7.12.0 milestone Oct 8, 2025

grantfitzsimmons requested a review from CarolineDenis October 8, 2025 01:04

grantfitzsimmons approved these changes Oct 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Decouple linting, formatting, and pre-commit from external dotfiles #7447

Decouple linting, formatting, and pre-commit from external dotfiles #7447

Uh oh!

mark-pitblado commented Sep 23, 2025 •

edited

Loading

Uh oh!

mark-pitblado commented Sep 24, 2025

Uh oh!

grantfitzsimmons left a comment •

edited

Loading

Uh oh!

mark-pitblado commented Oct 8, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	run: \|
	# TODO: Once most errors are fixed remove "set +e" to not ignore errors
	set +e
	npx eslint --fix --color `echo "${{steps.changed.outputs.changed}}" \| tr " " "\n"`
	set -e

Decouple linting, formatting, and pre-commit from external dotfiles #7447

Are you sure you want to change the base?

Decouple linting, formatting, and pre-commit from external dotfiles #7447

Uh oh!

Conversation

mark-pitblado commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency changes

Checklist

Testing instructions

Uh oh!

mark-pitblado commented Sep 24, 2025

Uh oh!

grantfitzsimmons left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark-pitblado commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mark-pitblado commented Sep 23, 2025 •

edited

Loading

grantfitzsimmons left a comment •

edited

Loading

mark-pitblado commented Oct 8, 2025 •

edited

Loading