chore: add overview of code quality tools; format code with black (DEV-2224) #391
Conversation
There was a problem hiding this comment.
I don't think this should exist in our documentation. It makes sense to note somewhere, which style we follow; but this seems way out of scope of what we should have in our documentation.
There was a problem hiding this comment.
I see your point. For me personally, it was important to make all these notes, and to store them somewhere where they can be accessed, and reevaluated. When the ecosystem will have changed in some years, and we don't have these notes, then it will be more difficult to assess if our choice of tools is still okay.
In my view, these notes make our choice more transparent.
And I would never have put these notes into the user part of the documentation. If they belong somewhere, then here in the developers/ADR section.
Writing this, the idea comes to my mind to move them to my personal github.io page. This would allow me to keep them stored somewhere, and to reference them, without cluttering the dsp-tools docs. What do you think?
There was a problem hiding this comment.
I wouldn't recommend mixing private and work stuff too much... but that's up to you. However if it's on your personal page, you shouldn't expect other team members to search for it there.
To me, the most intuitive place for this kind of internal note-keeping would be Notion, as it is our "internal Wiki".
But at the end of the day, it really doesn't matter too much; and you can also just leave it in the docs.
There was a problem hiding this comment.
again, this should not be in our documentation
| - Code quality tools: | ||
| - Overview: developers/code-quality-tools/code-quality-tools.md | ||
| - General formatting: developers/code-quality-tools/general-formatting.md | ||
| - Python formatting: developers/code-quality-tools/python-formatting.md | ||
| - Python docstring formatting: developers/code-quality-tools/python-docstring-formatting.md | ||
| - Python type checking: developers/code-quality-tools/python-type-checking.md | ||
| - Python linting: developers/code-quality-tools/python-linting.md | ||
| - Security checks: developers/code-quality-tools/security.md | ||
| - See also: developers/code-quality-tools/python-see-also.md |
There was a problem hiding this comment.
see my comments above. It is not the purpose of our documentation to describe the python ecosystem. Our documentation should be about our code/product, and to some extent on how this code is written/checked/etc. But anything beyond that just convolutes the docs and makes them less accessible and relevant.
| parser_process_files.add_argument("--nthreads", type=int, default=None, help="number of threads to use") | ||
| parser_process_files.add_argument("xml_file", help="path to XML file containing the data") | ||
|
|
||
| # upload-files | ||
| parser_upload_files = subparsers.add_parser( | ||
| name="upload-files", | ||
| help="For internal use only: upload already processed files" | ||
| help="For internal use only: upload already processed files", |
There was a problem hiding this comment.
this seems like a weird linting choice to me
There was a problem hiding this comment.
It seems weird at first glance, but after thinking more deeply about it, I began to find it cool. It reduces git diffs: If you add a new parameter, the diff will only highlight the single line that you added, because the comma on the old line is already there.
But I agree that black is opinionated (they characterize themselves as opinionated, but my opinion is very close to theirs, so I'm super happy 😄 )
| parser = argparse.ArgumentParser(description=f"DSP-TOOLS (version {version('dsp-tools')}, © {datetime.datetime.now().year} by DaSCH)") | ||
| subparsers = parser.add_subparsers(title="Subcommands", description="Valid subcommands are", help="sub-command help") | ||
| parser = argparse.ArgumentParser( | ||
| description=f"DSP-TOOLS (version {version('dsp-tools')}, © {datetime.datetime.now().year} by DaSCH)" | ||
| ) | ||
| subparsers = parser.add_subparsers( | ||
| title="Subcommands", description="Valid subcommands are", help="sub-command help" | ||
| ) |
There was a problem hiding this comment.
IMO this got less readable
There was a problem hiding this comment.
This is the only linting choice of black that I don't like. But since I love the rest, I will have to live with this here...
| from dsp_tools.models.propertyelement import \ | ||
| PropertyElement as PropertyElement # pylint: disable=useless-import-alias | ||
| from dsp_tools.models.propertyelement import PropertyElement as PropertyElement # pylint: disable=useless-import-alias | ||
| from dsp_tools.models.value import UriValue | ||
| from dsp_tools.utils.shared import \ | ||
| check_notna as check_notna # pylint: disable=useless-import-alias | ||
| from dsp_tools.utils.shared import \ | ||
| simplify_name as simplify_name # pylint: disable=useless-import-alias | ||
| from dsp_tools.utils.shared import check_notna as check_notna # pylint: disable=useless-import-alias | ||
| from dsp_tools.utils.shared import simplify_name as simplify_name # pylint: disable=useless-import-alias | ||
| from dsp_tools.utils.shared import validate_xml_against_schema |
There was a problem hiding this comment.
this is nice, though
jnussbaum
changed the title
resolves DEV-2224