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

Remove explicit import of lru-cache workaround #2587

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

peterpeterparker
Copy link
Member

Motivation

Following build reproducibility improvements resulting of #2584, #2574 and #2562, I think we do not need the explicit import of lru-cache workaround anymore.

I ran the ./script/test-reproducibility script which seems fine.

WDYT @dskloetd ?

@peterpeterparker peterpeterparker added the build Build related scripts, libraries and tooling label May 26, 2023
@peterpeterparker peterpeterparker requested a review from dskloetd May 26, 2023 09:32
@dskloetd
Copy link
Contributor

If I remember correctly, I was not able to reproduce the issue solved by this import locally but investigated it based on CI runs.
If we want to try this, we'll have to check with scripts/nns-dapp/ci-consistency after a while.
I'd rather first have a clean record following our recent effort.

@peterpeterparker peterpeterparker marked this pull request as draft May 26, 2023 09:40
@peterpeterparker
Copy link
Member Author

Sure works for me, let's postpone.
I'll keep the PR in a draft, ultimately I'm happy if we can remove explicit import at the root of the app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build related scripts, libraries and tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants