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.

Sign up for GitHub

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

Jump to bottom

Fixed #33 - Issue - Update the GuzzleHTTP package #34

Closed
wants to merge 2 commits into from
Closed

Fixed #33 - Issue - Update the GuzzleHTTP package #34

wants to merge 2 commits into from

Conversation

pratikrm
Copy link
Contributor

Closes #33

@pratikrm pratikrm self-assigned this Nov 14, 2023
@pratikrm pratikrm requested review from balazsrm and removed request for uzair-b November 14, 2023 17:41
balazsrm
balazsrm approved these changes Nov 15, 2023
View reviewed changes
Copy link
Collaborator

@balazsrm balazsrm left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I remember having some issue when I tried updating Guzzle the last time, something like a PHP compatibility issue. I don't remember exactly, and anyway this PR seems to work fine, so I'm approving it.

@pratikrm
Copy link
Contributor Author

I am afraid about the 2M+ changes, it increased the zip size by 6 MB

@balazsrm
Copy link
Collaborator

Now that you mention it, I remember we had the same issue even with the current version, and we solved it by deleting most of the unneeded code from the Google API library.

But that is probably not a great solution, because if another plugin uses the same Google API, and it's ours that gets loaded, then the missing code could cause problems on the site.

@pratikrm
Copy link
Contributor Author

@balazsrm, better if we use JavaScript or Node Library then:
https://github.com/google/google-api-javascript-client
https://github.com/googleapis/google-api-nodejs-client
?

@balazsrm
Copy link
Collaborator

balazsrm commented Oct 2, 2024

The Node library is not really an option, because that can’t be run on PHP-based WP sites. The JS library might work but it would probably require extensive changes in the plugin. I’ll check what can be done.

@balazsrm
Copy link
Collaborator

New PR: #41

@balazsrm balazsrm closed this Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@balazsrm balazsrm balazsrm approved these changes

@surajv surajv Awaiting requested review from surajv

Assignees

@pratikrm pratikrm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Issue - Update the GuzzleHTTP package
2 participants
@pratikrm @balazsrm