Maintainability upgrades and fixes#399
Maintainability upgrades and fixes#399Will-Hellinger wants to merge 18 commits intoComputerScienceHouse:developfrom
Conversation
mxmeinhold
left a comment
mxmeinhold
left a comment
There was a problem hiding this comment.
took a look through, though I found it hard to read through all the formatting. Glancing through commits, seemed like there wasn't much in code changes, more in deps and CI?
Have a couple comments
120 line length Single quotes
pikachu0542
left a comment
pikachu0542
left a comment
There was a problem hiding this comment.
Code looks good, need to run it locally to test
pikachu0542
left a comment
pikachu0542
left a comment
There was a problem hiding this comment.
I was able to get it running locally, and it works. However, I cant seem to figure out how to test the mock user functionality, and with no open packets, I cant verify that the main functionality still works as expected.
| - name: Lint with ruff and pylint | ||
| run: | | ||
| pylint packet/routes packet | ||
| ruff check packet && pylint packet/routes packet |
There was a problem hiding this comment.
nit: typically each linter/formatter is run as a separate stage since its easier to tell which one failed
| @@ -0,0 +1,33 @@ | |||
| name: Format App | |||
There was a problem hiding this comment.
This seems a little overengineered to have an auto format workflow especially since there is a format check part of the linting workflow already. I think it'd be better to let people fix the formatting themselves instead of a github bot committing to their branch automatically.
| """ | ||
| end_date = None | ||
|
|
||
| end_date: Union[datetime, None] = None |
There was a problem hiding this comment.
Can this (and all other cases like this) be an Optional[T]?
| # CSH Web Packet | ||
|
|
||
| [](https://www.python.org/downloads/release/python-390/) | ||
| [](https://www.python.org/downloads/release/python39/) |
There was a problem hiding this comment.
Why was this changed? The previous link was valid and this one is to a 404.
4 participants
Someone will need to double check this LOL