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

vsc_ast.py: Implement version_label #48

Merged
merged 1 commit into from
Jun 15, 2023
Merged

Conversation

gunnarx
Copy link
Collaborator

@gunnarx gunnarx commented Nov 23, 2022

As discussed in #44

@@ -483,6 +483,9 @@ class Namespace:
minor_version: Optional[int] = None
""" Provides the minor version of the namespace. """

version_label: Optional[str] = str()
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we discussed what to do with the syntax file now when we at least partially can generate documentation. Keep up-to-date for now, or let it degrade until we are ready to replace it with a generated version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Have we discussed what to do with the syntax file now when we at least partially can generate documentation. Keep up-to-date for now, or let it degrade until we are ready to replace it with a generated version?

The syntax file ought to be removed from the VSC repository since the IFEX documentation now provides all that information. The VSC project can optionally refer to IFEX for the definition of the language used for the service descriptions.

@miketsukerman
Copy link
Collaborator

miketsukerman commented Dec 2, 2022

Sorry, I haven't participated in the discussion and maybe this question has been clarified already. Only wanted to ask why can't we use simply

version: Optional[str] = str()

Where version has special format, for instance https://peps.python.org/pep-0440/#version-scheme. Instead of minor_version, major_version and version_label etc.

Signed-off-by: Gunnar Andersson <gunnar_dev@[email protected]>
@gunnarx
Copy link
Collaborator Author

gunnarx commented Apr 17, 2023

Rebased.

@gunnarx
Copy link
Collaborator Author

gunnarx commented Apr 17, 2023

Sorry, I haven't participated in the discussion and maybe this question has been clarified already. Only wanted to ask why can't we use simply

version: Optional[str] = str()

Yes, a standard version string containing <major>.<minor>.<patch> might be an option that gives us the major/minor meaning. I will leave it open for now but if someone really feels they want that as an alternative way to write it, then we could support both.

Where version has special format, for instance https://peps.python.org/pep-0440/#version-scheme. Instead of minor_version, major_version and version_label etc.

I generally like the idea of reusing an existing specification but:

  1. That particular specification is too complicated, in my opinion. If that spec is used then all cases need to be supported and supported correctly, or a complex explanation needs to be written to explain which features are used and what they mean.
  2. It is much simpler to support separate major and minor numbers and have client-server compatibility logic based on plain semantic versioning.
  3. Then, to allow any freely decided format for the extra-information string simplifies implementation further. Inside a particular company process or in some other special case, the string format might be defined in some way and local tools programmed to interpret it, but many standard tools will be able to just ignore it and that simplifies the implementation.

@gunnarx
Copy link
Collaborator Author

gunnarx commented Apr 17, 2023

NOTE! Optional patch version still missing. see #44

@gunnarx gunnarx merged commit 434300a into COVESA:master Jun 15, 2023
@gunnarx gunnarx deleted the version_label branch August 4, 2023 17:10
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