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

Validate ConanFileReference only if requested #3623

Merged
merged 15 commits into from Sep 27, 2018

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Sep 26, 2018

Closes #3509

This could speed things up a little bit if passing validate=False, current implementation add this flexibility while preserving existing behavior.

Changelog: Feature: Validate parameter for ConanFileReference objects to avoid unnecessary checks

@jgsogo jgsogo added this to the 1.9 milestone Sep 26, 2018
@jgsogo jgsogo requested a review from lasote September 26, 2018 11:27
@ghost ghost assigned jgsogo Sep 26, 2018
@ghost ghost added the stage: review label Sep 26, 2018
@@ -61,25 +61,29 @@ class ConanFileReference(namedtuple("ConanFileReference", "name version user cha
sep_pattern = re.compile("@|/|#")
revision = None

def __new__(cls, name, version, user, channel, revision=None):
def __new__(cls, name, version, user, channel, revision=None, validate=True):
Copy link
Member

Choose a reason for hiding this comment

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

Should validate defaulted to False and only validate in the ConanFileReference loading points?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel safer if we keep the previous behavior (always validating), at least in this pull request. Afterward, we can force it to be explicit and decide at each point if validation is needed or not.

@danimtb
Copy link
Member

danimtb commented Sep 27, 2018

Updated PR description with changelog. Feel free to change it if not accurate enough

@danimtb danimtb merged commit 2b2058b into conan-io:develop Sep 27, 2018
@ghost ghost removed the stage: review label Sep 27, 2018
@lasote lasote modified the milestones: 1.9, 1.8 Sep 27, 2018
@jgsogo jgsogo deleted the engineering/3509 branch September 27, 2018 15:29
grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018
* check that parameters are string before applying regex match

* add tests related to ConanName

* typo

* update docs related to function

* use six to check for string types

* use a custom message to validate each field

* refactor tests to validate custom messages

* fix tests: message is more explicit

* given conan-io#3464 this is dead code

* format error message only if we are actually going to use it

* minor changes: address @lasote review

* optional validation of conanfile reference
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.

None yet

3 participants