Conversation
* code formatting, linting etc * dependency management with poetry
chrisdevereux
force-pushed
the
docsite
branch
from
3c2c14e to
1b4864e
Compare
chrisdevereux
force-pushed
the
docsite
branch
from
1b4864e to
0f2a450
Compare
| hooks: | ||
| - id: isort | ||
| name: isort | ||
| entry: poetry run isort --settings-path pyproject.toml |
There was a problem hiding this comment.
Very cool. As using isort is a little obscure, though really useful, could we have a one line comment in each of these steps explaining roughly what we are doing?
docs/developing.md
Outdated
|
|
||
| We love development containers. They keep your development environment in a VM and isolated from the rest of your device which is good security practice. They also make it extremely easy to share editor configurations and development dependencies that can't be managed using language package managers. | ||
|
|
||
| Much as we hate to stan Microsoft, it's the easiest option to work on this library. |
There was a problem hiding this comment.
I'd link out to something here saying why Microsoft are bad news. Just to explain why.
docs/developing.md
Outdated
| - Python 3.9+ | ||
| - Poetry | ||
|
|
||
| * Follow the installation instructions for [DjangoGIS's local dependencies](https://docs.djangoproject.com/en/3.2/ref/contrib/gis/install/postgis/). |
There was a problem hiding this comment.
Probably want to use numbers not bullet points here for consistency.
docs/developing.md
Outdated
| ### In Visual Studio Code | ||
|
|
||
| 1. From the command palette select "Remote Containers: Clone Repository in Named Container Volume". | ||
| * Enter: `git@github.com:commonknowledge/pycommonknowledge.git`. |
There was a problem hiding this comment.
Probably want numbers, not bullet points for this list of instructions.
There was a problem hiding this comment.
Was doing this so that it autonumbers the list, but not wedded to it, and it's good to have more readable source
docs/index.md
Outdated
|
|
||
| # Pycommonknowledge | ||
|
|
||
| An integrated and opinionated collection of Django applications and javascript components addressing needs for people building software for organisers and campaigners. |
There was a problem hiding this comment.
Nitpick, but Javascript is a proper noun.
| <!--- Provide a general summary of your changes in the Title above --> | ||
|
|
||
| ## Description | ||
| <!--- Describe your changes in detail --> | ||
|
|
||
| ## Motivation and Context | ||
| <!--- Why is this change required? What problem does it solve? --> | ||
| <!--- If it fixes an open issue, please link to the issue here. --> | ||
| <!--- If there's a Figma file or similar spec, please link to to the issue. --> | ||
|
|
||
| ## Screenshots (if appropriate): | ||
|
|
||
| ## Types of changes | ||
| <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> | ||
| - [ ] Bug fix (non-breaking change which fixes an issue) | ||
| - [ ] New feature (non-breaking change which adds functionality) | ||
| - [ ] Breaking change (fix or feature that would cause existing functionality to change) | ||
|
|
||
| ## Checklist: | ||
| <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> | ||
| <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> | ||
| - [ ] I've checked the spec (e.g. Figma file) and documented any divergences. | ||
| - [ ] My code follows the code style of this project. | ||
| - [ ] My code follows the code style of this project. | ||
| - [ ] My change requires a change to the documentation. | ||
| - [ ] I've updated the documentation accordingly. | ||
| - [ ] Replace unused checkboxes with bullet points. |
There was a problem hiding this comment.
This template is inherited by the org-wide settings, so we don't need to include it in this repo. Duplicating it means we can craft it to fit this package's context better, but we lose automatic improvements from the upstream version. If we do duplicate, we could probably improve it by cutting line 24 (which is a duplicate!)
This should also be fixed in the org-wide version...
There was a problem hiding this comment.
I don't feel that the org-wide settings need changing – they look pretty good to me, although sure they'll evolve as we use them more.
Just felt that a library has slightly different requirements for a PR than an application. They change a bit more in the forthcoming contributor guidelines PR.
3 participants
Uh oh!
There was an error while loading. Please reload this page.