Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update dependency gonzales #83

Merged
merged 3 commits into from
Oct 17, 2023
Merged

Conversation

yuhsianw
Copy link
Collaborator

@yuhsianw yuhsianw commented Sep 26, 2023

Changes:

  • Update gonzales-pe to v4.3.0 which fixed a parsing bug and more.
  • Remove gonzales-primitives that was previously added as a bandaid for that parsing bug.

@yuhsianw yuhsianw force-pushed the frankwang/update-dependency-gonzales branch from 36e8ea2 to d361f8e Compare September 26, 2023 21:21
@yuhsianw yuhsianw force-pushed the frankwang/update-dependency-gonzales branch from d361f8e to 982b549 Compare September 26, 2023 21:34
@yuhsianw yuhsianw marked this pull request as ready for review September 26, 2023 21:36
@yuhsianw yuhsianw force-pushed the frankwang/update-dependency-gonzales branch from 982b549 to 9cdf401 Compare September 26, 2023 21:54
@yuhsianw yuhsianw added the dependencies Pull requests that update a dependency file label Sep 28, 2023
export default {
parse: (...args) => {
try {
return gonzales.parse(...args);
} catch (e) {
try {
return gonzalesPrimitive.parse(...args);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this no longer needed?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's mentioned in the PR description:

Update gonzales-pe to v4.3.0 which fixed a parsing bug and more.
Remove gonzales-primitives that was previously added as a bandaid for that parsing bug.

Copy link
Collaborator Author

@yuhsianw yuhsianw Sep 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Steve. To elaborate on that:

gonzalesPrimitive is an old gonzales-pe version build that was added by the previous maintainer, possibly to address some bugs introduced in the version of gonzales-pe used at that time (v4.0.3).

With the latest version gonzales-pe v4.3.0, a lot of bugs were fixed and the test case added was no longer failing. Therefore, I'm removing this fallback after upgrading to v4.3.0.

@yuhsianw yuhsianw merged commit 201a012 into master Oct 17, 2023
@yuhsianw yuhsianw deleted the frankwang/update-dependency-gonzales branch October 17, 2023 17:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants