Skip to content
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

Release - v1.1.1 #132

Merged
merged 10 commits into from
Dec 27, 2022
Merged

Release - v1.1.1 #132

merged 10 commits into from
Dec 27, 2022

Conversation

qgustavor
Copy link
Owner

@qgustavor qgustavor commented Dec 23, 2022

Release v1.1.1

Changelog

Patch Changes

Housekeeping (@qgustavor)

Rewrite tests to not leak async operations, removing the need of using sanitizeResources: false, and fix the test workflow.

Fix Zalgo in Storage class (@qgustavor)

  1. Added tests for checking for Zalgo. 2. Moved email check to .login() so it would not run in the constructor. 3. Added process.nextTick to avoid Zalgo. 4.

Fixing tests (@qgustavor)

Fixes #93, an issue related to crypto that showed up in tests and fixed Deno tests.

Better error handling in API (@qgustavor)

  1. Check response status before calling .json() and handle them properly. 2. Emit or ignore errors instead of throwing.

1. Check response status before calling .json() and handle them properly.
2. Emit or ignore errors instead of throwing.
Standard indented those lines but kept comments with original indentation. It's ugly!
* Better error handling in API

1. Check response status before calling .json() and handle them properly.
2. Emit or ignore errors instead of throwing.

* Fix linter related issue

Standard indented those lines but kept comments with original indentation. It's ugly!
@qgustavor
Copy link
Owner Author

qgustavor commented Dec 23, 2022

@JAForbes Sorry, but why the Publish step didn't finish? https://github.com/qgustavor/mega/actions/runs/3766660276/jobs/6403399581

@qgustavor
Copy link
Owner Author

Pre-release version published: https://www.npmjs.com/package/megajs/v/1.1.1-next.1

It would be wonderful if someone can test it. I will wait some time and an answer from JAForbes (maybe I set up pr-release wrongly) before merging into main.

@qgustavor
Copy link
Owner Author

Weird: I cancelled the job, then ran it again, it finished in 45 seconds. First time it took more than 10 minutes then I had to cancel it manually. A second pre-release with the crypto fixes is available in npm: https://www.npmjs.com/package/megajs/v/1.1.1-next.2

1. Added tests for checking for Zalgo
2. Moved email check to `.login()` so it would not run in the constructor
3. Added process.nextTick to avoid Zalgo
4. Added option to use additional arguments when testing (to enable debugging)
5. Improved createPromise implementation (it should compress better)
@JAForbes
Copy link

JAForbes commented Dec 23, 2022

Hey, not sure, it looks like you everything configured correctly. That step is this simple:

await $`npm publish --tag=${tag}`
// then node exits

I'll look into it but I'd probably say this is just a github actions bug. It looks like the package published, there's no reason for node to not exit at that point. I'm guessing it did exit but then the state didn't change in the actions runner.

I might add a flag to log any open handles in node and to log when node is about to exit because then you could at least rule this out as a pr-release issue.

Either way I think you can safely ignore this. But let me know if you see it again, I'll open an issue to track the handle logging.

@qgustavor
Copy link
Owner Author

I just tested the latest pre-release in a real application dealing with a lot of files and it went well. I will merge this PR in 24 hours if no one reports any issue until them.

@qgustavor qgustavor merged commit 291a432 into main Dec 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants