Skip to content

Add support for anchor idl fetch to work outside anchor workspace - #1509

Merged
paul-schaaf merged 5 commits into
otter-sec:masterfrom
MLH-Fellowship:cli-workspace
Mar 12, 2022
Merged

Add support for anchor idl fetch to work outside anchor workspace#1509
paul-schaaf merged 5 commits into
otter-sec:masterfrom
MLH-Fellowship:cli-workspace

Conversation

@NBNARADHYA

Copy link
Copy Markdown
Contributor

Context

Closes #1453 (Refer to this issue for context)

Changes

  1. anchor idl fetch can now be run outside an anchor workspace.
  2. If it is run outside workspace, provider.cluster option has to be provided.

Signed-off-by: Nikhil B N <nikhilbn365@gmail.com>
@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf I have made changes. Please suggest any changes if they are required.

Also had a question: How is anchor cli tested ? If I need to write tests for my change, how do I go about this?

Thanks

@paul-schaaf

paul-schaaf commented Feb 22, 2022

Copy link
Copy Markdown
Contributor

dont think we have to require the provider.cluster to be provided. if it's not given and we're outside a workspace, the cluster from the local solana config can be used

re:cli tests we..just test it manually. But we can start adding tests to the CI. Are you comfortable doing that or do you prefer us to do it? imo it's fine to just merge this without testing and then merge a ci change later but if you want to add tests to tests.yaml now, feel free!

@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf Thanks! On it!

If this works fine, I would prefer for this to be merged! I shall create a new PR with the tests if that's okay!

Thanks

@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf please suggest any changes!

@armaniferrante

Copy link
Copy Markdown
Contributor

@armaniferrante

please don't needlessly tag. Someone will review when they have time.

@paul-schaaf

Copy link
Copy Markdown
Contributor

looks like you found the cli-config solana crate but why did you copy that code into ours instead of importing the crate?

@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

looks like you found the cli-config solana crate but why did you copy that code into ours instead of importing the crate?

@paul-schaaf makes more sense! I have made changes

Comment thread cli/src/config.rs
Comment thread cli/src/lib.rs Outdated
Comment thread cli/src/lib.rs Outdated
@paul-schaaf

Copy link
Copy Markdown
Contributor

pls also add a line to the feature section in the changelog for the unreleased release

Signed-off-by: Nikhil B N <nikhilbn365@gmail.com>
@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf hope this is done!

@NBNARADHYA
NBNARADHYA requested a review from paul-schaaf March 3, 2022 19:39
@paul-schaaf

paul-schaaf commented Mar 7, 2022

Copy link
Copy Markdown
Contributor

pls update your changelog and if you do another PR, adjust your PR to allow edits from maintainers so I can just do that and merge instead

@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf I have already added updated the changelog. I am not able to see “allow edits from maintainers” option on this pull request.

@paul-schaaf

Copy link
Copy Markdown
Contributor

u need to update it again. your change is in the wrong place cause we created a release

@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf done !

@paul-schaaf

paul-schaaf commented Mar 8, 2022

Copy link
Copy Markdown
Contributor

pls sign your commits and force push again

…kspace

Signed-off-by: Nikhil B N <nikhilbn365@gmail.com>
@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf done!

@paul-schaaf

Copy link
Copy Markdown
Contributor

looks like your commits are still not signed

@NBNARADHYA
NBNARADHYA force-pushed the cli-workspace branch 3 times, most recently from 7d99492 to c5eec84 Compare March 8, 2022 20:40
@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf I can see that they are on github

@fanatid

fanatid commented Mar 9, 2022

Copy link
Copy Markdown
Contributor

@NBNARADHYA they are not, if commits are signed then you will see Verified label
Screenshot from 2022-03-09 11-08-37

@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@fanatid @paul-schaaf I used this command while commiting: git commit -m "message" -S. I believe this will sign commits. I am not sure what went wrong. BTW this is the output for git log --show-signature.
Screenshot from 2022-03-09 14-15-09

@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@fanatid Ok. Now I figured out I have to use GPG keys to sign. But i tried to edit those commits by rebasing but it edits all other commits and signs them all for some reason

@NBNARADHYA
NBNARADHYA force-pushed the cli-workspace branch 3 times, most recently from eeb30ff to c5eec84 Compare March 9, 2022 09:06
@NBNARADHYA
NBNARADHYA force-pushed the cli-workspace branch 6 times, most recently from eeb30ff to c5eec84 Compare March 9, 2022 17:28
@paul-schaaf

Copy link
Copy Markdown
Contributor

we've turned off the signing requirement. Just make sure you have no more conflicts and then Ill merge

@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf Great! Thanks. Just resolved the conflicts

@paul-schaaf
paul-schaaf merged commit 00488b5 into otter-sec:master Mar 12, 2022
@paul-schaaf

Copy link
Copy Markdown
Contributor

thank you for the contribution!

@NBNARADHYA

Copy link
Copy Markdown
Contributor Author

@paul-schaaf Thank you!

akash-osec pushed a commit to akash-osec/anchor that referenced this pull request Mar 25, 2026
@jamie-osec jamie-osec added this to the Pre-1.0 milestone Jun 24, 2026
akash-osec pushed a commit to akash-osec/anchor that referenced this pull request Jul 25, 2026
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.

cli: anchor idl fetch should work outside workspaces

5 participants