Skip to content

Subclass semver for nginx - #63

Merged
pombredanne merged 3 commits into
mainfrom
nginx-version
May 12, 2022
Merged

Subclass semver for nginx#63
pombredanne merged 3 commits into
mainfrom
nginx-version

Conversation

@pombredanne

Copy link
Copy Markdown
Member

Because nginx versions are not exactly semver.

Signed-off-by: Philippe Ombredanne pombredanne@nexb.com

@pombredanne pombredanne changed the title Sublcass semver for nginx Subclass semver for nginx Apr 26, 2022
* Because nginx versions are not exactly semver.
* Also add convenience properties to the semver version class to avoid
  leaking internals too much.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Comment thread src/univers/versions.py

@property
def major(self):
return self.value and self.value.major

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice trick !

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's more of an idiom to me! Though in earnest I am always split in abusing this shortcutting of "and" and "or" and their returning of a value as used here ... it can become hard to read unless used for very simple checks like this one.

@TG1999 TG1999 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@pombredanne
pombredanne merged commit 5b54577 into main May 12, 2022
@pombredanne
pombredanne deleted the nginx-version branch May 12, 2022 06:59
keshav-space pushed a commit that referenced this pull request Aug 30, 2023
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.

3 participants