Tech Meeting Notes 2020 05 28
Erik Moeller edited this page May 28, 2020
·
1 revision
GOAL: We decide what the style should be, we document, do in a single PR (ideally) into each repo, add to CI to prevent new style issues from being introduced
What should our code style be? Can we make this simple by adopting an autoformatter? If yes, what does the autoformatter not do that we should also standardize?
- black, 100 line length -> TODO: will need to file issues in each repository
- e.g. ordering of imports using isort
Some style guides / autoformatters for reference:
- Le goog - https://google.github.io/styleguide/pyguide.html (recommends to use yapf to autoformat code)
- Ethereum - https://github.com/ethereum/snake-charmers-tactical-manual/blob/master/style-guide.md
- black - https://github.com/psf/black
CONSENSUS: use of black sufficient for now
ACTION TAKEN: cross-repo adoption of black nominated for next sprint, starting with existing proxy PR.
- Allie: annoying to read code that's split because of line length limits. Often we read code vertically, and splitting some fragments into the next line can make that harder.
- Allie: Even 100 is not quite long enough IMO
- Erik: 100 seems like a reasonable compromise right now given accessibility concerns above it
CONSENSUS: use of 100 characters
- Limited team experience with isort so far, John has used it and is very happy with it
- General agreement that automating ordering would help if it can be done reliably John: If we want to add isort, we have to add additional config
CONSENSUS: Add isort to the client repo, see how we like it - maybe consider in future repos
ACTION TAKEN: exploratory use of isort nominated for next sprint
- We can use ignoreRevsFile to exclude those revisions from git-blame
- Can we make these policies uniform across all securedrop subprojects or do we need to special case?
- Can we centralize the configuration of all the tools? What's the state of .editorconfig, setup.cfg, pyproject.toml, etc?
- TODO: check pyproject.toml works nicely for our purposes, resolve in securedrop-proxy#61
- TODO: use git ignore revs file