-
Notifications
You must be signed in to change notification settings - Fork 422
Modernize code and improve readability #444
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
Conversation
|
Converting to draft until the code on main is sorted after #446. |
965b7ca to
1f0f200
Compare
0d4becd to
995f160
Compare
|
@addyosmani can you check the this PR and #403 and if everything looks good then cut a new minor version? This is what we have so far: 3.0.1...main |
@addyosmani friendly ping! |
|
@XhmikosR Yes! Apologies as my bandwidth is low at the moment. Would you prefer I review and land 403 first, 444, or no preference? |
|
@addyosmani no worries! Better land/review this PR and I can rebase #403 later :) |
|
@addyosmani friendly ping :) |
|
Minor: CI tests appear failing for me. The changes overall look reasonable but just want to double check that bit :) |
|
These errors happen on main too, they are random and running CI again fixes them. Not sure why, but I never managed to pinpoint the root cause... |
addyosmani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Thanks again, @XhmikosR! 🥳 |
This is an old patch I had lying around.
The size increase should be small, but I believe it makes code easier to read and also uses a few more modern features.
Non-whitespace diff: https://github.com/GoogleChromeLabs/quicklink/pull/444/files?w=1