-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add Code Linting and Implementation of Hypermodern Python #185
Merged
MRichards99
merged 53 commits into
feature/remove-sql-dependency-from-backends-#154
from
feature/add-code-linting-#165
Jan 4, 2021
Merged
Add Code Linting and Implementation of Hypermodern Python #185
MRichards99
merged 53 commits into
feature/remove-sql-dependency-from-backends-#154
from
feature/add-code-linting-#165
Jan 4, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- As per Hypermodern Python guide
- As suggested by Alan, a top level directory is required to make this repo ready for production use. This is also required for Poetry
…dencies - This will help keep consistent versions of flake8 etc being installed between developers, to prevent incosistent linting/safety outputs
- Done by `pre-commit run --all-files`
- These files have been replaced by the use of Poetry to store the API's dependencies
…anges on this branch
- This is so flake8 can correctly detect which import statements are bringing in local code, so said statements can be ordered in a consistent style
- This is the suggested line length defined by Black, which flake8 is configured to also follow
- S311 (from Bandit) states "Standard pseudo-random generators are not suitable for security/cryptographic purposes.". The generator script doesn't generate data that would be vulnerable to security (no keys or anything of that nature is created using the random library) hence this status code is ignored
- In production, this will be used to store the API's logs in /var/log/ - Also remove an unused variable that I noticed
…e-location-#182 Allow Log File Location to be Configurable
…aml-write-#183 Disable openapi.yaml Generation
…feature/add-code-linting-#165
…feature/add-code-linting-#165
VKTB
requested changes
Dec 3, 2020
Co-authored-by: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com>
- Also added a command to generate this tree in the future (I just used to update it manually).
…ties/datagateway-api into feature/add-code-linting-#165
- Most likely caused when I merged other branches after I created the PR
- The commit also includes a rebuilt openapi.yaml
2 tasks
- This fixes a PermissionError that was found when using these Nox sessions on Windows - This is a replacement solution for the tmp_dir option/fix, so that's now been removed
…feature/add-code-linting-#165
VKTB
approved these changes
Jan 4, 2021
…#184 Apply Fixes suggested by Linting Tools
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will close #165.
The related issue was originally meant for adding the capability for a code linter, but this branch has become an implementation of the Hypermodern Python guide (https://cjolowicz.github.io/posts/hypermodern-python-01-setup/). This means I've added code linting, Nox as a way of launching things easily (including unit tests when I finally get round to them), Poetry as an improved way of tracking the API's Python dependencies and making use of commit hooks using Pre Commit.
The README has a long section (https://github.com/ral-facilities/datagateway-api/tree/feature/add-code-linting-%23165#creating-dev-environment-and-api-setup) about how this all works, so instead of repeating myself here, going to look over there should explain how these changes work. That section should explain how to create a dev environment ready to start contributing code to this repo with some brief instructions on common usages for the tools.
There are a couple of things to keep in mind; the code has not been linted yet, this branch is just about adding the infrastructure. There's a big long list outputted by
flake8
so I think it's better to make the changes in a separate branch. For this reason, the pre commit hooks don't lint any code - I will add this once I've finished doing the linting (otherwise the hooks wouldn't let me commit anything!).This PR also makes a change suggested by @ajkyffin, which adds a directory which stores
common
andsrc
to make the code installable as a single Python package. As suggested, I've kepttest
andutil
outside of this directory because that's not code to be used in production.