mcstatus
mcstatus copied to clipboard
Add `clean` method to `motd` field
This will do easier to answer on questions like this or this.
I also will port this to #306 when will resolve merge conflict from master.
>>> status = JavaServer.lookup("hypixel.net").status()
>>> status.description
' §aHypixel Network §c[1.8-1.19]\n §d§l3RD SKYBLOCK ANNIVERSARY'
>>> status.description.clean()
' Hypixel Network [1.8-1.19]\n 3RD SKYBLOCK ANNIVERSARY'
Looks good to go, though it may be nice to add some unit-tests before merging into master
I'm sure, regex isn't a something, that should be tested. I firstly thought about adding parametrize, but this unnecessary. I will add one test just for coverage.
Perhaps we should take this further and make a class dedicated to parsing Minecraft text.
As I wrote in discord, this is just fast implement of one feature, that I come up with. I already made #335 for html
, xt256
, ansi
etc. colors and more complex MOTD parsing.
So it's approved, maybe merge it?
So it's approved, maybe merge it?
There was a requested change from @kevinkjt2000 which even though you commented on as addressed, kevin didn't yet submit an approving review, dismissing the requested changes review status. I like sticking to a policy of PRs having no requested changes from maintainers before going through with a merge, so even though I approved, I won't merge the PR until kevin makes an approving review too.
To be clear, I do see that #335 exists and I still believe that the new code should land in its already planned future home. Why would we add PingResponse.Description
to overhaul that immediately later, when we could add a new class that is later extensible in #335 for the rest of the planned methods?
To be clear, I do see that #335 exists and I still believe that the new code should land in its already planned future home. Why would we add
PingResponse.Description
to overhaul that immediately later, when we could add a new class that is later extensible in #335 for the rest of the planned methods?
Maybe it should be in #335, but #335 is blocked by #306, which is waiting for your review for almost 1.5 months. This feature has also been requested several times on Discord, so should we delay a small (and requested) change for so long?
For now, let's at least avoid putting a class inside of another class.
When editing legacy code, follow the legacy structure. I'll change it in #335 (after merging #306) to the new structure, but for now let's not create chaotic code.
When editing legacy code, follow the legacy structure.
No. By this same logic, I would never do chores. While living in a dirty house, keep living in a dirty house...
We have already moved some other nested classes away from being nested classes. Indeed, we should not add more. There was a lengthy discussion about how nesting classes is unpythonic in the past. # THIS IS SO UNPYTHONIC
is still in the code in fact.
which is waiting for your review for almost 1.5 months
I am slow to review, but at the moment I see the do-not-merge label and conflicts that need to be resolved on #306. Personally, I skip over PRs that still have changes outstanding from the checks that I see as an "automatic grader". This way I can optimize my time to review PRs that are ready. Also, this PR has conflicts too.
so should we delay a small (and requested) change for so long?
Yes, if that feature is not polished enough yet. Users expect quality even if they never ask for it. Open source does not have to be rushed, there is no money on the line here.
I am not saying to add all the planned functionality to that class yet. I am suggesting placing the class into its final destination from the start and avoiding rework.
Indeed, we should not add more.
I would like to fix it in specified for this place - #306. It would be worse to put it near PingResponse
class (instead of in it) and create some chaos in code.
Let's make the placement correct from the beginning... Why are you insisting on making a mess (that you already plan to cleanup) with the initial placement of this brand new class? I'm saying do that bit of cleanup now. Why wait?
Messy vs. Clean, clean wins.
Why wait?
Because if not follow current style, code will be messy more, than if we would follow the style. Even if it's ugly.
However, I will move this PR to #335. (oh wait, I did it already in Motd.plain
...)
oh wait, I did it already in Motd.plain...
Almost as if placing this code into the new class in the correct location was better in the first place... I think you are catching on :)