-
Notifications
You must be signed in to change notification settings - Fork 43
Port code to use ext-encoding #244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stable
Are you sure you want to change the base?
Conversation
this is a test run to see what it's like to work with the new API. So far, it seems to be OK, although I keep forgetting the parameters for the static methods, which is annoying. Instinctively I'm reaching for instance methods, and I have to keep reminding myself why I chose to implement the new API this way instead of using instance methods (it's because this design is more resistant to mistakes).
|
Is this completed? |
im curious too |
|
It seemed fine during integration testing, but I haven't touched it for a while. Will probably integrate it with the PM 5.40 release if all goes well. The rollout plan was to get BedrockProtocol live with it first so that if any weird problems showed up with ext-encoding, they'd be relatively easy to debug. That actually went much better than I expected, seems basically no one even noticed the change aside from a jump in performance, although that also led to me forgetting about it. So it's looking good for migrating other components too. |
this is a test run to see what it's like to work with the new API.
So far, it seems to be OK, although I keep forgetting the parameters for the static methods, which is annoying. Instinctively I'm reaching for instance methods, and I have to keep reminding myself why I chose to implement the new API this way instead of using instance methods (it's because this design is more resistant to mistakes).
Haven't tested this yet. Fine to ignore the test errors (it's just because the extension isn't present).