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 incorrect memoizations #7773

Merged
merged 1 commit into from
Aug 9, 2023
Merged

Fix incorrect memoizations #7773

merged 1 commit into from
Aug 9, 2023

Conversation

deivid-rodriguez
Copy link
Contributor

The fetch_file_if_present method returns nil if the file is not present. That means we can't use ||= for memoizing its result, and we'll end up retrying every time.

@deivid-rodriguez deivid-rodriguez requested a review from a team as a code owner August 9, 2023 12:14
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/memoization-fixes branch from 791cbd8 to a9c7c7b Compare August 9, 2023 12:14
@github-actions github-actions bot added L: php:composer Issues and code for Composer L: ruby:bundler RubyGems via bundler L: go:modules Golang modules L: elm Elm packages L: rust:cargo Rust crates via cargo L: dart:pub Dart packages via pub L: javascript labels Aug 9, 2023
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/memoization-fixes branch from a9c7c7b to 1073fc4 Compare August 9, 2023 15:11
@github-actions github-actions bot added L: terraform Terraform packages L: dotnet:nuget NuGet packages via nuget or dotnet L: python labels Aug 9, 2023
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/memoization-fixes branch from 1073fc4 to cc7c228 Compare August 9, 2023 15:17
@deivid-rodriguez
Copy link
Contributor Author

@landongrindheim I pushed more changes so that now all instances of this issue should be fixed!

@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/memoization-fixes branch from cc7c228 to ab4ebb4 Compare August 9, 2023 15:23
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/memoization-fixes branch from ab4ebb4 to e819a84 Compare August 9, 2023 15:36
@deivid-rodriguez deivid-rodriguez merged commit dcaa945 into main Aug 9, 2023
@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/memoization-fixes branch August 9, 2023 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@landongrindheim landongrindheim landongrindheim approved these changes

Assignees
No one assigned
Labels
L: dart:pub Dart packages via pub L: dotnet:nuget NuGet packages via nuget or dotnet L: elm Elm packages L: go:modules Golang modules L: javascript L: php:composer Issues and code for Composer L: python L: ruby:bundler RubyGems via bundler L: rust:cargo Rust crates via cargo L: terraform Terraform packages
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@deivid-rodriguez @landongrindheim