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

Tooling is somewhat poorly discoverable #1692

Closed
EliahKagan opened this issue Oct 4, 2023 · 0 comments · Fixed by #1693
Closed

Tooling is somewhat poorly discoverable #1692

EliahKagan opened this issue Oct 4, 2023 · 0 comments · Fixed by #1693

Comments

@EliahKagan
Copy link
Contributor

The Running Tests section of the readme includes some information about tools, such as linters, to use with the project, how to run them, and where their configuration files are. However, this has not been updated in a while, and:

  • CI is the primary way tools are run for this project, but the CI workflows are not mentioned.
  • pre-commit is used for several checks, but its own configuration file is not mentioned.
  • Information on preconditions for running tests in forks is outdated: CI runs on all branches, so the fork need not have a main branch. (In practice, forks will, but the key is that people do not need to develop their features on their fork's default branch.)

The bigger issue, though, is how recent changes from pull requests I have opened have made the situation worse:

  • tox is not mentioned in the readme.
  • tox.ini defines an html environment to test building documentation, but does not list it in env_list, so it is hard to notice it exists. The reason it is not listed is that it should not be run by default, because it writes (and overwrites) files in the actual doc/build directory. If doc/Makefile is made to support custom build directories, then that tox environment can be made to write documentation into its temporary directory, and it can be listed (and otherwise treated) like the other tox environments.
  • tox.ini specifies Python 3.9 for everything that doesn't need to run on multiple versions of Python, for compatibility with Cygwin, and it allows missing interpreters to be skipped without error, also for compatibility with Cygwin. But users may well not have Python 3.9, in which case these just don't run, and may not be noticed. tox.ini should not multiply those environments for multiple Python versions, but it should list backup versions (every other supported version, in some reasonable order) as well, for those environments to use.
  • Users can run black ., but black is not being run on CI, nor via pre-commit, which makes it easy to forget to run it or, if one is less familiar with the project's tooling, to be unaware that it is even available.
  • Every time I have made a change to a shell script in this project, I have run shellcheck. But as far as I know, I have not even mentioned this tool anywhere, only alluded to its use. This is not quite an issue of the tool's discoverability: most people who contribute to this project probably won't edit a shell script, but when someone does, it would be useful for shellcheck to run on CI and (when in use) via pre-commit to make possible problems with the changes discoverable.

In addition, it is arguably a discoverability issue that flake8 is not listed as a development dependency (which is not necessary for running it via pre-commit, which takes care of installing it in a virtual environment it manages). However, I am still reluctant to add it and its plugins as development dependencies, because I think much older versions with possibly different behavior would be gotten on Python 3.7, and because I still intend to replace it with ruff.

Through my work on these issues so far, I have come to think all this could be fixed together and in the same PR that would also fix #1690 and #1691. However, in case the scope turns out to be too large, I'm making this issue so the matter is not forgotten about. Having this also has the benefit that there is less to document in a PR that covers it along with other things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant