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

fix: remove ProgramVersion circular dependency #145

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

johnrees
Copy link
Contributor

While setting up tests I noticed there's a circular dependency which is causing some unexpected behaviour.

Calls to ProgramVersion.V1 will sometimes fail because ProgramVersion === undefined when there are circular dependencies with enums

before

Screenshot 2021-12-16 at 10 32 21 PM

after

Screenshot 2021-12-16 at 10 31 57 PM

@netlify
Copy link

netlify bot commented Dec 16, 2021

✔️ Deploy Preview for sad-einstein-a108a8 ready!

🔨 Explore the source changes: 35e3747

🔍 Inspect the deploy log: https://app.netlify.com/sites/sad-einstein-a108a8/deploys/61bbbef05019310008c62a81

😎 Browse the preview: https://deploy-preview-145--sad-einstein-a108a8.netlify.app

@netlify
Copy link

netlify bot commented Dec 16, 2021

✔️ Deploy Preview for wizardly-kowalevski-4abe4b ready!

🔨 Explore the source changes: 35e3747

🔍 Inspect the deploy log: https://app.netlify.com/sites/wizardly-kowalevski-4abe4b/deploys/61bbbef0658fc30008b30719

😎 Browse the preview: https://deploy-preview-145--wizardly-kowalevski-4abe4b.netlify.app

@johnrees
Copy link
Contributor Author

These red nodes are the files linked to scripts/governance-notifier.ts which are affected by circular imports

graph

and after

graph

I think I will add the borsh fix in this PR too

@johnrees
Copy link
Contributor Author

nvm, I'll come back to borsh in a separate PR if it's an issue

@SebastianBor SebastianBor merged commit 06207f5 into solana-labs:main Dec 17, 2021
@johnrees johnrees deleted the circular-deps branch December 17, 2021 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants