Only check the touched markdown files in PR for the Verify link step

[sima-zhu]

Java: Azure/azure-sdk-for-java#16008
.NET: Azure/azure-sdk-for-net#15929
Python: Azure/azure-sdk-for-python#14466
JS: Azure/azure-sdk-for-js#11803
Android: Azure/azure-sdk-for-android#405
C: Azure/azure-sdk-for-c#1431
CPP: #787
Go: Azure/azure-sdk-for-go#12809
IOS: Azure/azure-sdk-for-ios#488

[ahsonkhan]

What's motivating this change? Was this causing problems somehow?

[sima-zhu]

What's motivating this change? Was this causing problems somehow?

Most of the lang repo are lack of the ability to scan the root readme (markdown outside of sdk scope). So every time people add wrong link under root repo, only the nightly build aggregate-report can catch it. We want to minimize the two steps fix to one step.
Also, there is no need to scan the links if the PRs do not touch the markdown files.

[ahsonkhan]

So every time people add wrong link under root repo, only the nightly build aggregate-report can catch it.

Why is that?

We want to minimize the two steps fix to one step.

How does it minimize the number of steps? Do we still have the nightly build that verifies all the links? If not, don't we lose some coverage?

If the repo somehow is in a state that it contains invalid links, are we still going to be able to catch that somehow, regardless of this change?

[weshaggard]

@sima-zhu it doesn't look like the cpp repo has an aggregate-reports pipeline like other language repos so @ahsonkhan is correct that for cpp this will leave a gap because we will never check the links unless something is touching that MD file in a PR. So for languages where we don't have a nightly pipeline runs and checks all MD files, we should either add one or keep checking all MD files.

[sima-zhu]

@weshaggard @ahsonkhan I am trying to check over if cpp have the ability to scan the root readme. However, it seems we cannot scan root readme for cpp as well. I think the best approach to address this is we add a nightly pipeline to check all links like other lang's aggregate-report.

[ahsonkhan]

Thanks @weshaggard, that makes sense. I would still like to understand the benefit/concern of this change for other languages that might already have a nightly aggregate report.

However, it seems we cannot scan root readme for cpp as well.

What's the underlying reason for this limitation (both in C++ and other languages)?

I think the best approach to address this is we add a nightly pipeline to check all links like other lang's aggregate-report.

OK.

[sima-zhu]

@ahsonkhan This is one of the readme we currently miss to scan against in PR. https://github.com/Azure/azure-sdk-for-cpp/blob/master/README.md
Other langs have the same issue.

[sima-zhu]

ci.yml provides us which sdk directory (storage, core etc) to scan against. The link verification scripts scan the markdown files under sdk directory parameter. However, we do not have any ci.yml cover the path outside of sdk scope. That's why we have the miss.

[ahsonkhan]

Gotcha. Can we change the ci.yml script (or add a new one at the root) to include the root directory too or is there a reason we want to exclude them?

For instance, can we add something like *, here:

azure-sdk-for-cpp/sdk/core/ci.yml

Lines 11 to 14 in 12b7a5e

	- cmake-modules/
	- eng/
	- CMakeLists.txt
	- sdk/core

[weshaggard]

Unfortunately devops doesn't really have support wildcards (https://docs.microsoft.com/en-us/azure/devops/pipelines/repos/github?view=azure-devops&tabs=yaml#wildcards) for paths so if we added the root of the repo as a trigger I think it would trigger for every change in the repo, not just the files in the root. So I'm not sure we want to do that and we don't do it in any of the other languages today either.

We could however have one or all the pipelines in the cpp repo check all MD files in the repo that change but it would require that they get triggered by other changes or manually triggered to check them which is what this change is about.

I think perhaps the best option is to keep this change add to add another nightly aggregate-reports pipeline, like all other languages, that runs and will always check all MD files for broken links.

[ahsonkhan]

Unfortunately devops doesn't really have support wildcards (https://docs.microsoft.com/en-us/azure/devops/pipelines/repos/github?view=azure-devops&tabs=yaml#wildcards) for paths so if we added the root of the repo as a trigger I think it would trigger for every change in the repo, not just the files in the root.

That is certainly a strange limitation. Looks like / would work, but like you said, that would include everything.

I think perhaps the best option is to keep this change add to add another nightly aggregate-reports pipeline, like all other languages, that runs and will always check all MD files for broken links.

👍

[sima-zhu]

The aggregate-report pipeline has to setup for all necessary checking against entire repo. It should take more things into consideration besides of link verification. We currently plan to scan the entire repo in yml file, which is able to cover all md files for the very first step. If it takes too much time for scanning, we will switch to another plan. Will close the PR and open another one for covering root readme