archinstall
archinstall copied to clipboard
Fix mypy complaints in models.dataclasses.py
🚨 PR Guidelines:
New features (v2.2.0)
All future work towards v2.2.0 is done against master now.
Any patch work to existing versions will have to create a new branch against the tagged versions.
Describe your PR
I fixed all of the complaints from mypy in models.dataclasses.py. This was in attempt to help the library be more compliant with PEP 561 as discussed in issue #114.
You'll notice that in the comparison dunder functions, I replaced the type from VersionDef to object. This is because these methods can expect any type of object, which is why I raise NotImplemeted if it gets compared with any other type than itself. Inspired by this: https://stackoverflow.com/questions/54801832/mypy-eq-incompatible-with-supertype-object
Testing
In another commit I wrote a few unit tests for the VersionDef class. That being said I noticed that there isn't a preexisting test suite yet (that's in another branch, presumably coming soon). So feel free to disregard that commit if you want.
I'm not sure why, but this PR breaks packages.py.. They shouldn't have any correlation but yet it breaks?

@demipaki If this file is fully mypy compliant now, could you add it to the github action hook as well https://github.com/svartkanin/archinstall/blob/66765e1735e950a6d40c6d44c5615b065dc1d309/.github/workflows/mypy.yaml#L18-L18? This way it will remain clean since nobody can make modifications anymore that are not compliant :)
I've just pushed commits to this branch by accident without doing the test, that broke packages.py, that @Torxed mentioned...
I'll request another review once I've done the test myself. Sorry, this is my first time contributing to a project on github :sweat_smile:
No worries hehe, and critique sounds so harsh :P More a gentle nudge ^^ I appreciate the PR!