-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add checksums API #22
Conversation
This has different behavior than the WP checksums API for versions that aren't found: https://api.wordpress.org/core/checksums/1.0/?version=4.9.99&locale=en_US returns |
This seems to work exactly as required. I've carried out extensive testing on an upgrade from APIs used for testingCP 1.1.4: https://api-v1-test.classicpress.net/checksums/1.1.4.json ResultThe upgrade copied 275 files in total as shown here (arranged in alphabetical order for convenience): Show files
To cross-check this, I used the command Error checkingTo test what happens when an invalid API URL is used, I set the "new" API to https://api-v1-test.classicpress.net/checksums/4.9.15.json. Although this returned a 404, the upgrade continued but copied all files instead of just the ones that had changed. |
Thank you for checking @timbocode. It sounds like this is doing what it's supposed to do, but I am still not clear on the exact conditions required for a file not to be copied/upgraded as it should be. Does this happen on any upgrade from v1.1.4 to v1.2.0, or is an earlier version also required to be installed before v1.1.4? I am also thinking it would be a good idea to add another path component here for the type of checksum being used. This would mean that https://api-v1-test.classicpress.net/checksums/1.1.4.json becomes https://api-v1-test.classicpress.net/checksums/md5/1.1.4.json, and we'd have a natural place to put checksums using other hash algorithms in the future. |
Also, a minor improvement that shouldn't block us getting the initial version out, but the |
3a07c67
to
26ef196
Compare
Looks to work properly from my quick testing. Closes #16.