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: Add keys include/exclude in programs section #546

Merged
merged 6 commits into from
Jul 26, 2021

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Jul 23, 2021

Not sure that this is correct but I think it would be good if programs directory would be able to contain shared code (crate) between programs. Right now anchor cli consider all directories in in programs as program and trying to compile / generate IDL. This PR allows to include / exclude directories.

@armaniferrante
Copy link
Member

Let's add this to one of the examples/tests so that we don't regress the feature. You can use examples/misc if there's no obvious place to put it.

@fanatid
Copy link
Contributor Author

fanatid commented Jul 24, 2021

@armaniferrante I added include/exclude to examples with different combinations.

@@ -1,3 +1,7 @@
[provider]
cluster = "localnet"
wallet = "~/.config/solana/id.json"

[programs]
Copy link
Member

@armaniferrante armaniferrante Jul 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of renaming this to [workspace], similar to Cargo?

I'm a little hesitant to use the [programs] name because in the future we may want to combine the [[test.genesis]] and [clusters.<network>] sections into one named [programs], since they both allow customizing various features of programs in the workspace and feels a little more appropriate.

Copy link
Contributor Author

@fanatid fanatid Jul 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's will be better, thanks for the suggestion! Changed. But now I think, should [workspace] have members & exclude as Cargo (with members default to programs/* for example)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me! members may be better since it's consistent with Cargo but I'm happy with either choice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went ahead and did the rename.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I thought to add regex support but was busy yesterday.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be cool to see in a subsequent PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Should we use default value for members as["programs/*"]?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's hold off on the regex support. I'm making some changes that will conflict with this.

Copy link
Member

@armaniferrante armaniferrante Aug 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To support package publishing, I've changed the members and exclude arrays to take paths relative to the workspace root (instead of relative to the programs/ dir, as is done in this PR), which is consistent with how Cargo handles this.

The next version will this PR in it. But the immediate release following will contain the breaking change (PR here #570).

@armaniferrante armaniferrante merged commit b8dae74 into coral-xyz:master Jul 26, 2021
@fanatid fanatid deleted the cli-include-exclude branch July 26, 2021 04:39
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.

2 participants