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

cli: fixes bug where the wrong exit status is reported #2370

Merged
merged 2 commits into from Feb 6, 2023

Conversation

RuvenSalamon
Copy link
Contributor

Fixes: #2117

@vercel
Copy link

vercel bot commented Jan 25, 2023

@RuvenSalamon is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

@RuvenSalamon RuvenSalamon mentioned this pull request Jan 25, 2023
@Henry-E
Copy link
Contributor

Henry-E commented Jan 25, 2023

Ok, so this is linked to how AVM processes CLI calls and sends them to whatever version of anchor is active.

Don't forget to add a changelog.

@RuvenSalamon
Copy link
Contributor Author

RuvenSalamon commented Jan 25, 2023

Ok, so this is linked to how AVM processes CLI calls and sends them to whatever version of anchor is active.

Don't forget to add a changelog.

Done!

@Henry-E
Copy link
Contributor

Henry-E commented Jan 25, 2023

Dumb question but how can i verify / test that this doesn't just break AVM. I'm not sure if avm has tests or if the tests use avm for their CLI stuff. I only ask because I assume this is something you've tested locally in order to verify that it works?

@RuvenSalamon
Copy link
Contributor Author

RuvenSalamon commented Jan 27, 2023

Dumb question but how can i verify / test that this doesn't just break AVM. I'm not sure if avm has tests or if the tests use avm for their CLI stuff. I only ask because I assume this is something you've tested locally in order to verify that it works?

I basically compiled and ran the executable from the target dir. I do see some tests executing the anchor command in the tests dir (although it seems to use anchor from the ts package and not the avm wrapper) but no tests for avm in the avm dir.
I can add a test if you want, not sure if you'd like them in the tests folder as that seems to also function as a sort of repository of examples and this would not really be an example. I could create a tests dir in the avm dir?
I will be away during the weekend so might be slow to react. Will fix the conflict as well (unless somebody else wants to do these things)

@RuvenSalamon
Copy link
Contributor Author

(Accidentally closed the PR, misclick)

@Henry-E Henry-E marked this pull request as ready for review February 6, 2023 12:21
@Henry-E Henry-E merged commit 010b400 into coral-xyz:master Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong exit status
2 participants