ogr
ogr copied to clipboard
Enforce strict Optional typing
Some of these fixes are not perfect, in a few places I added Optional
where I feel it shouldn't really be. However, it's not really possible any other way without significant refactoring which would break the API (which I don't want to do). Sometimes a specific forge may return None
so it was necessary to update the type of the abstract class.
I tried to not make any functional changes, most of these are just to align the typing with the actual implementation.
TODO:
- [ ] regenerate some requre responses (the failing tests). I may need some help with this, I managed to do this for one of the failing tests but not for the repo creation yet.
Fixes #696
Related to #251
Merge before/after
Build failed.
:heavy_check_mark: pre-commit SUCCESS in 1m 59s :x: ogr-tests-rpm FAILURE in 6m 09s :x: ogr-tests-pip-deps FAILURE in 6m 26s :heavy_check_mark: ogr-reverse-dep-packit-tests SUCCESS in 13m 05s
I'll have a look.
BTW it kinda bothers me that you need to regenerate data after typing-related changes :D
Oh, only 4 tests? I expected worse :grin:
I've resolved some of the issues pointed out in the review, however I am still a bit unsure about the correct solution in some of the weird cases.
Build failed.
:heavy_check_mark: pre-commit SUCCESS in 2m 41s :x: ogr-tests-rpm FAILURE in 7m 07s :x: ogr-tests-pip-deps FAILURE in 7m 05s :heavy_check_mark: ogr-reverse-dep-packit-tests SUCCESS in 14m 09s
I've resolved some of the issues pointed out in the review, however I am still a bit unsure about the correct solution in some of the weird cases.
I think one of the issues with types is that we were using some of the »abstract« classes in the test cases all over the codebase of packit(-service), which makes the typing more complicated.
IMO in ideal case we should have everything hidden behind a property, so that we can handle it from specific git-forge and maybe have a „dummy“ classes for tests. However that will involve breaking of a lot of rev-dep tests and seems, at least to me, time-consuming to fix everywhere :/
This issue has been marked as stale because it hasn't seen any activity for the last 60 days.
Stale issues are closed after 14 days, unless the label is removed by a maintainer or someone comments on it.
This is done in order to ensure that open issues are still relevant.
Thank you for your contribution! :unicorn: :rocket: :robot:
(Note: issues labeled with pinned or EPIC are never marked as stale.)
This issue has been marked as stale because it hasn't seen any activity for the last 60 days.
Stale issues are closed after 14 days, unless the label is removed by a maintainer or someone comments on it.
This is done in order to ensure that open issues are still relevant.
Thank you for your contribution! :unicorn: :rocket: :robot:
(Note: issues labeled with pinned or EPIC are never marked as stale.)