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

Make docs.ms link relative link #3221

Merged
3 commits merged into from
Apr 27, 2022
Merged

Make docs.ms link relative link #3221

3 commits merged into from
Apr 27, 2022

Conversation

@sima-zhu sima-zhu self-assigned this Apr 26, 2022
@sima-zhu sima-zhu added the Central-EngSys This issue is owned by the Engineering System team. label Apr 26, 2022
@sima-zhu sima-zhu requested a review from danieljurek April 26, 2022 17:45
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

Copy link
Member

@danieljurek danieljurek left a comment

Choose a reason for hiding this comment

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

Can we get an example of this running and updating links? I think this can be accomplished by setting SetDevVersion to true and queuing a build for a service pipeline.

eng/common/scripts/Update-DocsMsMetadata.ps1 Outdated Show resolved Hide resolved
Co-authored-by: Daniel Jurek <djurek@microsoft.com>
@sima-zhu
Copy link
Contributor Author

sima-zhu commented Apr 27, 2022

Can we get an example of this running and updating links? I think this can be accomplished by setting SetDevVersion to true and queuing a build for a service pipeline.

The test is in description.

@sima-zhu sima-zhu requested a review from danieljurek April 27, 2022 17:56
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

Copy link
Member

@danieljurek danieljurek left a comment

Choose a reason for hiding this comment

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

LGTM!

I think one small corner case still exists here: someone uses a language region-language code is used here that isn't en-us ... We could try to match against a list of possible codes but I don't think the effort needed to generate and maintain such a list would be worth it in the scope of this effort.

Searching through the Java codebase reveals that other language codes are not in use in our README.md files.

Furthermore those errors would be flagged as warnings in OPS and could be addressed on a case by case basis if we start seeing more pop up.

@weshaggard
Copy link
Member

I thought about the other languages as well but based on our link guidance folks shouldn't even have the /en-us in the link either so I think we are probably ok here and we probably can even remove that from the regex if we wanted but I'm fine keeping as well.

@@ -129,13 +129,12 @@ author: $author
ms.author: $msauthor
ms.date: $date
ms.topic: reference
Copy link
Member

Choose a reason for hiding this comment

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

Why were these removed?

Copy link
Contributor Author

@sima-zhu sima-zhu Apr 27, 2022

Choose a reason for hiding this comment

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

This is according the their metadata guidance. If we use the ms.service, then the set of ms.prod and ms.technology are not necessary.
https://review.docs.microsoft.com/en-us/help/contribute/metadata-attributes?branch=main

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are many OPS warning regarding we are using wrong ms.prod value

@sima-zhu
Copy link
Contributor Author

I think we should leave the other locale as it is, so people can redirect to the right language as they defined.

@ghost
Copy link

ghost commented Apr 27, 2022

Hello @azure-sdk!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 7226450 into Azure:main Apr 27, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Central-EngSys This issue is owned by the Engineering System team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants