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

[chore] upgrades html-minifier-terser@5.0.0 -> 6.0.2 #1688

Merged

Conversation

@gabrielcsapo
Copy link
Contributor

@gabrielcsapo gabrielcsapo commented Sep 29, 2021

Summary

fixes #1687.

Updates the snapshots to reflect the terser upgrade.

@gabrielcsapo gabrielcsapo marked this pull request as draft Sep 29, 2021
@gabrielcsapo gabrielcsapo marked this pull request as ready for review Sep 29, 2021
@gabrielcsapo
Copy link
Contributor Author

@gabrielcsapo gabrielcsapo commented Sep 29, 2021

@jantimon I will have a follow up PR to make these fixtures snapshots as this would have been easier to run jest -u instead of having to update the fixtures manually if that okay with you?

@jantimon
Copy link
Owner

@jantimon jantimon commented Sep 30, 2021

You can rebuild all examples with npm run rebuild-examples

Snapshots are way harder to read

@gabrielcsapo
Copy link
Contributor Author

@gabrielcsapo gabrielcsapo commented Sep 30, 2021

I will run that to make sure I got everything! @jantimon

Is there anything else I should do before we can merge?

Copy link
Owner

@jantimon jantimon left a comment

We don't need async await to return the promise.
Can you please revert those two changes?

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@jantimon
Copy link
Owner

@jantimon jantimon commented Oct 9, 2021

@gabrielcsapo did you already have time to look into my feedback? :)

gabrielcsapo and others added 3 commits Oct 9, 2021
Co-authored-by: Jan Nicklas <j.nicklas@me.com>
Co-authored-by: Jan Nicklas <j.nicklas@me.com>
@gabrielcsapo
Copy link
Contributor Author

@gabrielcsapo gabrielcsapo commented Oct 9, 2021

@jantimon just updated the PR based on the feedback.

@gabrielcsapo
Copy link
Contributor Author

@gabrielcsapo gabrielcsapo commented Oct 13, 2021

@jantimon kindly bumping this.

@gabrielcsapo gabrielcsapo requested a review from jantimon Oct 13, 2021
@jantimon jantimon merged commit 16a841a into jantimon:main Oct 15, 2021
1 check passed
@jantimon
Copy link
Owner

@jantimon jantimon commented Oct 15, 2021

thanks @gabrielcsapo

@jantimon
Copy link
Owner

@jantimon jantimon commented Oct 15, 2021

released as 5.4.0

@jantimon
Copy link
Owner

@jantimon jantimon commented Oct 16, 2021

@gabrielcsapo your changes break the build pipelines - could you please take a look what’s wrong?

@gabrielcsapo
Copy link
Contributor Author

@gabrielcsapo gabrielcsapo commented Oct 16, 2021

@gabrielcsapo gabrielcsapo deleted the gabrielcsapo/upgrade-html-minifier-terser branch Oct 16, 2021
@gabrielcsapo
Copy link
Contributor Author

@gabrielcsapo gabrielcsapo commented Oct 16, 2021

@jantimon there seems to be a race condition with checking the contents of the files on disk and starting a new test. It might make sense to have each test output to a specific directory on disk to avoid collisions. I am not seeing this same issue locally, so it will be hard to debug to be for sure.

Another option is using snapshots as they are in memory and won't have collision issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants