Red-DiscordBot icon indicating copy to clipboard operation
Red-DiscordBot copied to clipboard

ISort, various new checks, problem matchers and more - configs

Open Jackenmen opened this issue 3 years ago • 1 comments

Type

  • [x] Bugfix
  • [x] Enhancement
  • [ ] New feature

Description of the changes

List of changes to be completed...

Notes:

  • make stylediff reports diffs separately
    • Previously I wrote a tool for running both tools on the files and showing the combined diff but I think it's just an unnecessary maintenance burden. Here's a link in case someone is interested in this work though: https://github.com/Cog-Creators/Red-DiscordBot/commit/563b94a17e0077398893b6cafbc6fc17d00e2a18
    • I was also considering removing make stylediff completely since it was primarily added for use in CI but it isn't much of a burden to at least keep it in this form and it might be useful for some people's workflows. pre-commit usage should, however, be preferred.

TODO:

  • re-enable flake8 for tests/ after figuring out a proper way to minimize the issues with the imports
  • Incorporate pylint too? Maybe it would be better as separate PR once pylint 2.7.3 gets released; TBD
  • Add a new recipe to Makefile that creates a new venv and auto-installs pre-commit as part of it
    • basically make newenv + pre-commit install combined
    • alternatively, we could just print a message in make newenv after it finishes everything else
  • add the following commits from #4894 once it's merged:
    • "Address issues from flake8"
    • "Reformat with Black 22.1.0"
    • "Reformat with isort 5.10.1"
    • "Remove trailing whitespace"
    • "Make files end with one empty line"
    • "Reformat JSON files with pretty-format-json hook"

Things that I have considered but ended up not doing:

  • a Makefile recipe that calls pre-commit run --all-files
    • note: this would have been a single recipe and it would not be the same as reformat, stylecheck, or stylediff as not all hooks are able to automatically reformat the files
    • I think that there isn't much use for this, pre-commit works better as an installed pre-commit hook, it already runs in CI, and it isn't particularly hard for people to run pre-commit run --all-files if that's their preferred workflow
  • auto-install pre-commit as part of newenv recipe in Makefile
    • it is not a good idea to force this workflow on people

The reformatting needed by this PR is done in #4894 and that PR should be merged before this one.

Link to problem matcher preview: https://github.com/Cog-Creators/Red-DiscordBot/commit/6378fdf45de03e201663ee22b423dd4944504da9 (can also be seen on the "Files changes" page in "Unchanged files with check annotations" section)

Fixes #3882, fixes #4698

Blocked by #4894 (the one that actually does the reformatting)

Jackenmen avatar Mar 15 '21 00:03 Jackenmen

If someone's interested in the diffs (do mind that this is still draft until the split and possibly some other changes), I suggest looking at the first 14 commits for all the configuration files: https://github.com/Cog-Creators/Red-DiscordBot/pull/4890/files/332b5296dce8c81de3b4dbab3d8e6b8263ca1b7a

Next 10 commits is all the reformatting effort (some of which was done by the tools automatically and the rest of the issues were addressed manually): https://github.com/Cog-Creators/Red-DiscordBot/pull/4890/files/332b5296dce8c81de3b4dbab3d8e6b8263ca1b7a..d21032b99e5b2782686cbef1806598300c25a0fb

The last commit is .git-blame-ignore-revs file which will need to be updated after the commits with all the reformatted stuff get in V3/develop: 59fbe37 (#4890)

Jackenmen avatar Mar 15 '21 00:03 Jackenmen