-
Notifications
You must be signed in to change notification settings - Fork 302
obviousAlexC/newgroundsIO: use minified JS instead #2339
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: master
Are you sure you want to change the base?
Conversation
|
Does this count as a logic change under |
|
yes you would want to test that it still works because i actually didn't do that |
|
if you can look at the repository where the JS is coming from, look at the commits that added them and convince yourself that the minified and non-minified JS files were generated at the same time and should work identically, then you can get away with not a lot of testing and still be probably confident that nothing was broken |
I'll just leave this to someone else, I think. |
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.
- I tested all modified functionality and found no bugs or unintuitive behavior.
- I believe a well-formed project using an older version of this extension will not be broken by this new version.
- I have no additional concerns.
- This change is kinda trivial and small but I tried
I clicked through most of the blocks to see if they would report or respond as expected and not throw any errors. It works but I could not do an in-depth test because I don't use Newgrounds.
DogeisCut
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.
- I tested all modified functionality and found no bugs or unintuitive behavior.
- I believe a well-formed project using an older version of this extension will not be broken by this new version.
- I have no additional concerns.
I'm not sure if a game has to be published for the API to work, but it seems all the blocks in the Save data section (except for the hat) return empty strings. It's a similar situation with the medals, and scoreboard, as despite providing valid ids into both, and successfully connecting to the game and logging in, they didn't seem to do anything. Results are inconclusive and would need larger testing from a more Newgrounds savvy reviewer.
Despite this, I believe due to the small change, that this is likely just how this extension has always worked, and is unrelated to the change to a minified version. However, I will not be approving until I can get some confirmation on this behavior.
You just need to be connected with a login prompt verified. Then all blocks should work. |
Saves some space. It should be easy to verify that the JS file was generated at the same commit as the unminified JS so you don't need to do crazy testing.