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

Feature: allow arbitrary defines in conan new templates #8718

Merged
merged 3 commits into from Mar 29, 2021

Conversation

SSE4
Copy link
Contributor

@SSE4 SSE4 commented Mar 28, 2021

allow arbitrary defines in conan new templates

e.g. instead of running:

$ conan new zip/0.1.30 -m cci.cmake

I may run:

conan new zip/0.1.30 -m cci.cmake -d license=Unlicense

which allows further automation on top of conan new, for instance, some script may fetch license, description, topics, homepage, tarball URL and sha256 in order to pass em to the template.

Changelog: Feature: Allow arbitrary defines in conan new templates.
Docs: conan-io/docs#2051

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

Signed-off-by: SSE4 <tomskside@gmail.com>
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Needs tests, please add.

conans/client/cmd/new.py Outdated Show resolved Hide resolved
@@ -199,6 +199,7 @@ def new(self, *args):
help='Generate a .gitignore with the known patterns to excluded')
parser.add_argument("-ciu", "--ci-upload-url",
help='Define URL of the repository to upload')
parser.add_argument('-d', '--define', action='append')
Copy link
Member

Choose a reason for hiding this comment

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

Sure this doesn't need the Extender custom action?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, I am not sure I understand, what do you mean... could you possibly describe your question in more details, e.g. with some examples?

Copy link
Member

Choose a reason for hiding this comment

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

We have Extender class in our command.py file:

class Extender(argparse.Action):

It is basically there to implement the append action it seems. Is the Extender class legacy code not needed? If that is the case maybe we can get rid of it? (Not in this PR, of course, in another one, but just wondering if the action='append' could have some limitation or problem)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems so, at least append exists in 2.7, so Extender seems to be a redundant bicycle for me, could be removed in conan 2.0

Copy link
Member

Choose a reason for hiding this comment

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

If it is really redundant, then let's drop it today, for next 1.35? Of course if we are relatively sure this will not be breaking users, but if we are, no need to wait until Conan 2.0.

This looks good, then, merging.

Signed-off-by: SSE4 <tomskside@gmail.com>
Signed-off-by: SSE4 <tomskside@gmail.com>
@SSE4
Copy link
Contributor Author

SSE4 commented Mar 29, 2021

@memsharded added test

@@ -199,6 +199,7 @@ def new(self, *args):
help='Generate a .gitignore with the known patterns to excluded')
parser.add_argument("-ciu", "--ci-upload-url",
help='Define URL of the repository to upload')
parser.add_argument('-d', '--define', action='append')
Copy link
Member

Choose a reason for hiding this comment

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

We have Extender class in our command.py file:

class Extender(argparse.Action):

It is basically there to implement the append action it seems. Is the Extender class legacy code not needed? If that is the case maybe we can get rid of it? (Not in this PR, of course, in another one, but just wondering if the action='append' could have some limitation or problem)

@memsharded memsharded added this to the 1.35 milestone Mar 29, 2021
@memsharded memsharded merged commit 9106369 into conan-io:develop Mar 29, 2021
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