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

locale (en-us) links go undetected in source code #20057

Closed
maorleger opened this issue Jan 25, 2022 · 13 comments
Closed

locale (en-us) links go undetected in source code #20057

maorleger opened this issue Jan 25, 2022 · 13 comments
Labels
EngSys This issue is impacting the engineering system.

Comments

@maorleger
Copy link
Member

I was looking into any instances where we have a locale link (en-us in the URL) and noticed that there are many such links in source code. I know our aggregate pipelines currently detect and warn against such links in markdown files (with a warning: DO NOT include locale /en-us/ information in links) - I believe it's a miss that we currently do not validate source code, although maybe there are technical reasons I am not aware of?

@ghost ghost added the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Jan 25, 2022
@maorleger maorleger changed the title locale links go undetected in source code locale (en-us) links go undetected in source code Jan 25, 2022
@weshaggard
Copy link
Member

We have a similar issue in .NET repo Azure/azure-sdk-for-net#22216

The biggest issue is we don't have a great way to find all the links in a JS file. Parsing the entire text and looking for links might be overkill as there might be lots of code that looks like it is a link but isn't. If you can help come up with a strategy that allows us to find the list of links we could then feed them into our link-verify script and have them validated.

@jeremymeng
Copy link
Contributor

We could probably do some minimum work via eslint rule? like matching strings having both https://docs.microsoft.com/ and en-us

@maorleger
Copy link
Member Author

maorleger commented Jan 25, 2022

@jeremymeng I think that's a great idea! it'll be flagged in PRs and easy to skip per-line due to any false-positives. Although I would say anything with http:.*/en-us/.* (we can refine it a bit of course) we can flag as we have many MDN links which also can contain locale information. That might get us like 80-90% of the way there to be honest which is progress...

@weshaggard
Copy link
Member

While checking en-us is good but I assume these will turn into real links in the ref docs so we should try and figure out a way to check that the links are valid.

@weshaggard
Copy link
Member

As part of our builds don't we produce the ref docs as html? If so we could pass the output of that directory to our link verification script that should flag any issues with the links in the generated documentation.

@maorleger
Copy link
Member Author

@weshaggard we do produce ref docs in PR builds 👍 do you think something like this would be feasible then?

Example: https://dev.azure.com/azure-sdk/public/_build/results?buildId=1322757&view=artifacts&pathAsName=false&type=publishedArtifacts where I pulled docs under packages -> {package-name} -> documentation

@weshaggard
Copy link
Member

Yes I do think that is possible and worth a try. See https://github.com/Azure/azure-sdk-for-js/blob/main/eng/pipelines/templates/steps/analyze.yml#L38 where we verify the links of the MD files. We should be able to have a similar step that points to the ref docs output and runs across those files.

@ramya-rao-a
Copy link
Contributor

@weshaggard In that case, should we move this to the azure-sdk-tools repo assuming this approach would apply to all languages?

@weshaggard
Copy link
Member

I would expect that each language would need to plugin the step independently based on their doc generation process. From a central EngSys prospective I think all the pieces are available.

@ramya-rao-a ramya-rao-a added the EngSys This issue is impacting the engineering system. label Feb 4, 2022
@ghost ghost removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Feb 4, 2022
@ramya-rao-a ramya-rao-a added this to the [2022] March milestone Feb 4, 2022
@ramya-rao-a
Copy link
Contributor

Thanks @weshaggard

@JonathanCrd, Please work with @praveenkuttappan in the March milestone for this task

@jeremymeng jeremymeng modified the milestones: [2022] March, [2022] April Mar 9, 2022
@JonathanCrd
Copy link
Member

While checking en-us is good but I assume these will turn into real links in the ref docs so we should try and figure out a way to check that the links are valid.

@weshaggard are you suggesting to validate that the URLs work after checking that they don't contain the locale information?

I mean, check for 2 things: validity and locale?

@weshaggard
Copy link
Member

Yes the work here is to run the verify-links script across all the generated html docs and that will verify that the links are both valid links and follow our guidelines like don't contain "en-us", etc.

@JonathanCrd JonathanCrd modified the milestones: [2022] April, [2022] May Apr 13, 2022
@xirzec xirzec modified the milestones: [2022] May, [2022] June May 17, 2022
@xirzec xirzec modified the milestones: [2022] June, [2022] July Jun 13, 2022
@xirzec xirzec modified the milestones: 2022-07, 2022-08 Jul 8, 2022
@JonathanCrd JonathanCrd removed their assignment Jul 12, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-js that referenced this issue Aug 2, 2022
reorder DeploymentResourceConfiguration Definition (Azure#20057)
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-js that referenced this issue Aug 5, 2022
[Hub Generated] Review request for Microsoft.MachineLearningServices to add version preview/2022-06-01-preview (Azure#19842)

* Adds base for updating Microsoft.MachineLearningServices from version stable/2022-05-01 to version 2022-06-01-preview

* Updates readme

* Updates API version in new specs and examples

* Add idleShutdownSetting to Compute Instances (Azure#19748)

* Added idleTimeBeforeShutdown to compute instance properties

* Added api UpdateIdleShutdownSetting

Co-authored-by: Teo Magnino Chaban <teom@microsoft.com>

* Add mfe.json 2022-06-01-preview changes (Azure#19740)

* update mfe.json with 2022-06-01-preview spec

* update go.md with preview-2022-06

* add AutoMLJob examples (initial)

* add LabelingJob examples (initial)

* add Schedule examples (initial)

* prettier

* update AutoMLJob examples

* update LabelingJob examples

* update LabelingJob examples

* prettier

* typo fixes

* add Vectorizer to custom-words.txt

* add missing

* is this a breaking change?

* Revert "update go.md with preview-2022-06"

This reverts commit b49c64ae0eb258069251c8c13085fd1e5dde56e2.

* Updating Swagger Spec for MLC for CustomServices (Azure#19781)

* Updating Swagger Spec for MLC for CustomServices

* Added Examples

* Removing update API in favor of exisiting Patch API and Fixing some Validation issues

* Fixed Pretty Issue

* Fixed Id Issue

* Changed case

* Updated Examples

* Changing Description

* Added nullable fields

* Added update APIs

* Fixing validation errors

* Fix model validation

Co-authored-by: Srivatsa Sinha <srsinha@microsoft.com>

* Add encryption property to workspace update parameters (Azure#19893)

* Add encryption property to workspace update parameters

Adding the ability for users to update CMK keyIdentifier property for workspace update.

* Apply prettier changes

Co-authored-by: Joshua Loeffler <jloeffler@microsoft.com>

* Add enum value to OutputDataDeliveryMode (Azure#19964)

* add soft delete related properties (Azure#19907)

* add soft delete related properties

* resolve lint diff issue that enum name duplicaton

Co-authored-by: Xi Jin <jinxi@microsoft.com>

* Remove SoftDeleteEnabled. (Azure#20005)

* reorder DeploymentResourceConfiguration Definition (Azure#20057)

Co-authored-by: Teddy Todorov <forteddyt@gmail.com>
Co-authored-by: teochaban <teochaban@hotmail.com>
Co-authored-by: Teo Magnino Chaban <teom@microsoft.com>
Co-authored-by: Teddy Todorov <thtodoro@microsoft.com>
Co-authored-by: srivatsasinha <102133347+srivatsasinha@users.noreply.github.com>
Co-authored-by: Srivatsa Sinha <srsinha@microsoft.com>
Co-authored-by: jloeffler7 <102193043+jloeffler7@users.noreply.github.com>
Co-authored-by: Joshua Loeffler <jloeffler@microsoft.com>
Co-authored-by: XI JIN <giovannijin1990@gmail.com>
Co-authored-by: Xi Jin <jinxi@microsoft.com>
Co-authored-by: ZhidaLiu <86350902+ZhidaLiu@users.noreply.github.com>
@xirzec xirzec modified the milestones: 2022-08, 2022-09 Aug 9, 2022
@xirzec xirzec modified the milestones: 2022-09, 2022-12 Nov 1, 2022
@xirzec xirzec modified the milestones: 2022-12, 2023-02 Jan 11, 2023
@xirzec xirzec removed this from the 2023-02 milestone Mar 31, 2023
Copy link

Hi @maorleger, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 13, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
EngSys This issue is impacting the engineering system.
Projects
None yet
Development

No branches or pull requests

6 participants