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

[chore] Update check-links workflow to match contrib logic #5728

Merged

Conversation

TylerHelmuth
Copy link
Member

Description:
Quick PR to closer align Core's checklink workflow with Contribs. I am hoping it fixes issues like this: https://github.com/open-telemetry/opentelemetry-collector/runs/7452564621?check_suite_focus=true.

@TylerHelmuth TylerHelmuth requested review from a team and Aneurysm9 July 21, 2022 17:17
@codecov
Copy link

codecov bot commented Jul 21, 2022

Codecov Report

Merging #5728 (9740a16) into main (61c6989) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #5728   +/-   ##
=======================================
  Coverage   91.56%   91.56%           
=======================================
  Files         192      192           
  Lines       11411    11411           
=======================================
  Hits        10449    10449           
  Misses        768      768           
  Partials      194      194           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61c6989...9740a16. Read the comment docs.

@bogdandrutu
Copy link
Member

Description:
Quick PR to closer align Core's checklink workflow with Contribs. I am hoping it fixes issues like this: https://github.com/open-telemetry/opentelemetry-collector/runs/7452564621?check_suite_focus=true.

[just for me to understand the logic]: Can you please explain how? That to me looks like a problem in the tool.

@TylerHelmuth
Copy link
Member Author

TylerHelmuth commented Jul 22, 2022

That to me looks like a problem in the tool.

I don't think so. In the build I linked the check-links job fails because there is a bad link in docs/release.md. The command being run is exactly

markdown-link-check \
    --verbose \
    --config .github/workflows/check_links_config.json \
    CHANGELOG.md docs/release.md

So the job is correctly running against the 2 files it was told to and finding a bad link in one of them. The problem is that my branch did not modify the docs/release.md file, so it shouldn't get passed to markdown-link-check.

In the "changed files" job the runs before the check-links job, the git diff command that checks what files changes is git diff --name-only --diff-filter=ACMRTUXB origin/main f34584b43df46ad8afa134184311cd45ec07bd31 | grep .md$ | xargs. When I run that locally I get CHANGELOG.md and docs/release.md.

If I run this PR's version of the git diff command for that PR instead, it only returns CHANGELOG.md, which is correct.

@TylerHelmuth
Copy link
Member Author

@djaglowski is the git wizard who fixed this issue in Contrib. Comparing against the common ancestor using git merge-base instead of directly against main is the key.

@bogdandrutu bogdandrutu merged commit b463d2a into open-telemetry:main Jul 27, 2022
@TylerHelmuth TylerHelmuth deleted the align-checklinks-with-contrib branch July 27, 2022 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants