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

[doc] Update manifest to reference versioning spec #17549

Merged
merged 7 commits into from Apr 29, 2021
Merged

[doc] Update manifest to reference versioning spec #17549

merged 7 commits into from Apr 29, 2021

Conversation

mathisloge
Copy link
Contributor

@mathisloge mathisloge commented Apr 28, 2021

Currently only version-string is included in the manifest-files.md. However there are more variants of this field. This PR adds a cross link to the versioning specification.

fixes #17548

Currently only `version-string` is included in the manifest-files.md. However there are more variants of this field. This PR adds a cross link to the versioning specification. 

fixes #17548
@JackBoosY JackBoosY added the category:documentation To resolve the issue, documentation will need to be updated label Apr 28, 2021
docs/maintainers/manifest-files.md Outdated Show resolved Hide resolved
@NancyLi1013 NancyLi1013 changed the title Update manifest to reference versioning spec [doc] Update manifest to reference versioning spec Apr 28, 2021
@mathisloge
Copy link
Contributor Author

@NancyLi1013 maybe we should remove the whole #### "version-string" section and just add the link to the specification?
I don't think it's really maintainable or useful to have the information duplicated.

@NancyLi1013
Copy link
Contributor

@NancyLi1013 maybe we should remove the whole #### "version-string" section and just add the link to the specification?
I don't think it's really maintainable or useful to have the information duplicated.

I agree with your suggestions. This section seems outdated. But since it is about version naming, we cannot remove it directly. We can rewrite this section. What do you think?

@mathisloge
Copy link
Contributor Author

I was wondering if the changes introduced with 66619e0 are sufficient. Since all details are available in the specification (with examples) and the needed information, which fields exists are given in the table added with the mentioned commit

docs/maintainers/manifest-files.md Outdated Show resolved Hide resolved
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
@mathisloge
Copy link
Contributor Author

@NancyLi1013 the "thumbs up" is the indication that we will drop #### "version-string" in favor of 66619e0 ? 😄

@NancyLi1013
Copy link
Contributor

@NancyLi1013 the "thumbs up" is the indication that we will drop #### "version-string" in favor of 66619e0 ? 😄

Currently, it might be not. I think we should rewrite this section. But I have no idea how to make it more reasonable. We have supported several kinds of version, only listing version-string here seems not good. The current changes look good to me. #### "version-string" section should be discussed.

Do you have any suggestion?

Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

Further tweaks and consolidation is needed, but this is certainly an incremental improvement.

@mathisloge
Copy link
Contributor Author

@NancyLi1013 From my point of view, the table with the possible fields would be sufficient. Since, in my opinion, all the information we would write here would be duplicated with the specification.
In my case, as someone who only updates or creates ports, this would be perfectly sufficient for me, since I know that there are other fields than version-string and I know where to look for examples and detailed information (in the linked spec.)

Maybe first merge these changes and discuss further changes in discussions or in a seperate PR?

@ras0219-msft ras0219-msft merged commit 4de6cd5 into microsoft:master Apr 29, 2021
@ras0219-msft
Copy link
Contributor

Thanks for the PR!

@mathisloge mathisloge deleted the patch-1 branch April 29, 2021 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:documentation To resolve the issue, documentation will need to be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update docs manifest
4 participants