search

Azure / azure-sdk-tools

Tools repository leveraged by the Azure SDK team.
MIT License
112 stars 172 forks source link

Link verification check error message is confusing #1439

Closed pakrym closed 3 years ago

pakrym commented 3 years ago

Sample output:

Here are all markdown files we need to check based on the changed files:
    /mnt/vss/_work/1/s/sdk/objectanchors/Azure.MixedReality.ObjectAnchors/CHANGELOG.md
    /mnt/vss/_work/1/s/sdk/objectanchors/Azure.MixedReality.ObjectAnchors/README.md
    /mnt/vss/_work/1/s/sdk/objectanchors/Azure.MixedReality.ObjectAnchors/src/autorest.md
Found 0 links on page file:///mnt/vss/_work/1/s/sdk/objectanchors/Azure.MixedReality.ObjectAnchors/CHANGELOG.md
Found 22 links on page file:///mnt/vss/_work/1/s/sdk/objectanchors/Azure.MixedReality.ObjectAnchors/README.md
##[warning][404] broken link https://github.com/Azure/azure-sdk-for-net/tree/0fe00c70af81e60ab232093e8dac3402ce575a8d/sdk/mixedreality/Azure.MixedReality.ObjectAnchors
Found 0 links on page file:///mnt/vss/_work/1/s/sdk/objectanchors/Azure.MixedReality.ObjectAnchors/src/autorest.md
Summary of broken links:
'file:///mnt/vss/_work/1/s/sdk/objectanchors/Azure.MixedReality.ObjectAnchors/README.md' has 1 broken link(s):
  https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/mixedreality/Azure.MixedReality.ObjectAnchors
##[error]Found 22 links with 1 page(s) broken.
##[error]PowerShell exited with code '1'.

There are two links in the output, the actual one used in the .MD file (https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/mixedreality/Azure.MixedReality.ObjectAnchors) and the processed one (https://github.com/Azure/azure-sdk-for-net/tree/0fe00c70af81e60ab232093e8dac3402ce575a8d/sdk/mixedreality/Azure.MixedReality.ObjectAnchors). Only the processed one is highlighted in DevOps output and it's confusing to customers because they don't have that exact URL in the MD file.

I propose to change the output to ##[warning][404] broken link https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/mixedreality/Azure.MixedReality.ObjectAnchors (resolved to https://github.com/Azure/azure-sdk-for-net/tree/0fe00c70af81e60ab232093e8dac3402ce575a8d/sdk/mixedreality/Azure.MixedReality.ObjectAnchors)

sima-zhu commented 3 years ago

This is by design.

Since some of links only exists in PR not in mater, we scan against the link in commits only. The error message comes from checking against the link with SHA commit. 

##[warning][404] broken link https://github.com/Azure/azure-sdk-for-net/tree/0fe00c70af81e60ab232093e8dac3402ce575a8d/sdk/mixedreality/Azure.MixedReality.ObjectAnchors

To change it to master, people will not realize the broken links are from the commit. Instead, they think it is normal there is no master link as the code has not checked in yet.

pakrym commented 3 years ago

Can we include both the original and the resolved link in the error message?

sima-zhu commented 3 years ago

@pakrym I will go ahead and have a clear message as you suggested.

sima-zhu commented 3 years ago

Another complaints I recently got is the log message is a little confusing: image

People kinda of misread the message to "Found 30 errors" instead of links. Maybe we can remove the successful log to debug level.

weshaggard commented 3 years ago

Even if we move that to the verbose logging I'd want to turn that on by default as it is good information and shows progress when there is a lot of links being checked. I'd be open to different wording but I don't think we should remove it or demote it to verbose/debug only output.