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

fix: remove create cache on every call #37

Merged
merged 3 commits into from
Apr 26, 2024
Merged

fix: remove create cache on every call #37

merged 3 commits into from
Apr 26, 2024

Conversation

rishtigupta
Copy link
Contributor

@rishtigupta rishtigupta commented Apr 26, 2024 •
edited
Loading

PR Description:

  • Remove Create Cache operation on every call
  • Update Readme

Testing:

  • Updated laravel-example repo to point to this branch
  • Created cache using pre-prod console
  • Use that cache in .env file and run the docker container to verify successful response from the OpenWeather API

Issue:

https://gomomento.slack.com/archives/C01AC7JT370/p1714111485262089

@rishtigupta rishtigupta marked this pull request as ready for review April 26, 2024 17:47
pgautier404
pgautier404 reviewed Apr 26, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
@rishtigupta rishtigupta requested a review from pgautier404 April 26, 2024 18:02
poppoerika
poppoerika reviewed Apr 26, 2024
View reviewed changes
Copy link
Collaborator

@poppoerika poppoerika left a comment •
edited
Loading

Choose a reason for hiding this comment

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

https://github.com/momentohq/laravel-cache/pull/37/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R35

Do we need to update this part manually?
WonderPlanet was using v 1.0.4 probably because they copied/pasted this part of the README.

@rishtigupta
Copy link
Contributor Author

https://github.com/momentohq/laravel-cache/pull/37/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R35

Do we need to update this part manually? WonderPlanet was using v 1.0.4 probably because they copied/pasted this part of the README.

Yes, I will update the readme to point to the latest version once this pr is merged

@rishtigupta rishtigupta requested a review from poppoerika April 26, 2024 18:05
poppoerika
poppoerika approved these changes Apr 26, 2024
View reviewed changes
Copy link
Collaborator

@poppoerika poppoerika 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

@rishtigupta rishtigupta merged commit a145ed2 into main Apr 26, 2024
1 check passed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poppoerika poppoerika poppoerika approved these changes

@pgautier404 pgautier404 pgautier404 approved these changes

@cprice404 cprice404 Awaiting requested review from cprice404

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rishtigupta @pgautier404 @poppoerika