mcstatus
mcstatus copied to clipboard
Rewrite answer objects
This rewrites answer classes in Public API.
Current Breaking Changes:
- Validation (from
PingResponse
) is now raisingTypeError
instead ofValueError
if the given type was not correct.
Migration steps
- TODO. It will be written when PR is merged. Now it is very fast changes.
TODO
- [x] Rename base classes with
Base
prefix. - [x] Move and write tests.
- [x] Actually, do we need tests?
- [x] Rewrite tests to use factory, currently there are too many duplicated code that can be avoided.
- [x] Write description for all attributes in classes, so users will not look for list of attributes with
dir
or looking in parents. - [ ] Write migration steps (after PR's merge).
- [ ] What must be done after the deprecation period (after PR's merge).
do-not-merge
label
This PR is too long not reviewed, so after fast conversation in discord, we thought about add do-not-merge
label and bump date for removing deprecations right before merging this PR.
Deprecation period will be ~6 months after the merge.
Not To Forget: Discuss and make backwards competitive.
We should also consider adding both java and bedrock terms as properties in the base response classes.
Java terms slight archaic, I saw many MOTD plugins and all of them never called MOTD as description. I'm sure, will be enough just wrote how was renamed fields in change-log and merge-guide.
The only problem of my backport is different signature in __init__
method. I tried do it with typing.overwrite
, but it only produce more problems.
Maybe I made too many parametrizing, because at now in master
there is 159 tests, and in this PR 425.
I hope no one (@ItsDrike, @kevinkjt2000) forgot about this PR, because last my reminder was in discord, two weeks ago. And no activity since it.
AFAIK the PR is still awaiting the review from @kevinkjt2000, I will not request any more changes/approve the PR until that happens.
@PerchunPak My life has been super busy lately. I reviewed approximately half of this 2 weeks ago. At the earliest, I will finally have time on Wednesday evening to finish reviewing.
Looks like last commits didn't run any CI, is this a bug? I also found one, when ran tests while resolving merge conflict.
EDIT: Runs now, maybe just logs was deleted.
I just noticed that 2022-08 is coming soon. Of course even if #306 will be merged now, half a month isn't a very long time to delete the deprecations. What will be the new time?
Adding do-not-merge label, as discussed on discord.
This PR can still be reviewed and approved, but shouldn't be merged before all tasks mentioned in the PR's description are addressed. For now, the only blocking task is this:
- [ ] Update the deprecation date to ~5 months after this PR is expected to get merged. Since this can only be done once all other issues are resolved and the PR gets approved, which can still potentially take a long time, we can't know what this date should be just yet. Needs do-not-merge so we don't forget to update this date later.
I added some description about attributes in f7086e8828909e57c885ea8c734f17e8b8a55d43, but it really should be replaced by link to API documentation from Sphinx.
I hope this fixes the main problem of the inheritance - you can't exactly know what attributes in class, by viewing source code.