Skip to content

build: Switch to mdlint cli2 - #8632

Open
thompson-tomo wants to merge 14 commits into
open-telemetry:mainfrom
thompson-tomo:mdlint-cli2
Open

build: Switch to mdlint cli2#8632
thompson-tomo wants to merge 14 commits into
open-telemetry:mainfrom
thompson-tomo:mdlint-cli2

Conversation

@thompson-tomo

@thompson-tomo thompson-tomo commented Jul 18, 2026

Copy link
Copy Markdown

Switch to a maintained markdownlint docker image/gh-action.

Note the new violation has been suppressed.

The scheduled check has been changed to mirror semconv and provide link to the action. This approach enables the github annotations to be visible hence improving usability etc.

@codecov

codecov Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.1%. Comparing base (0b71d86) to head (ccdd20e).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #8632   +/-   ##
=====================================
  Coverage   84.1%   84.1%           
=====================================
  Files        331     331           
  Lines      26468   26468           
=====================================
+ Hits       22268   22275    +7     
+ Misses      3795    3787    -8     
- Partials     405     406    +1     

see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thompson-tomo
thompson-tomo marked this pull request as ready for review July 18, 2026 06:02
Comment thread .github/workflows/markdown-fail-fast.yml Outdated
Comment thread .github/workflows/markdown.yml Outdated

@MrAlias MrAlias left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The linter migration is not ready to replace the existing checks yet: the fail-fast path is unreliable, and the current head remains red.

Comment thread .github/workflows/markdown-fail-fast.yml Outdated
Comment thread .github/workflows/markdown-fail-fast.yml Outdated
Comment thread .lycheeignore Outdated
http://jaeger-collector
https://github.com/open-telemetry/opentelemetry-go/milestone/
https://github.com/open-telemetry/opentelemetry-go/projects
https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file is parsed as regular expressions, so each unescaped $ is an end-of-line anchor instead of a literal workflow variable. The link checker is currently requesting both copies of this URL and failing with 404.

Suggested change
https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID
https://github.com/\$GITHUB_REPOSITORY/actions/runs/\$GITHUB_RUN_ID

@thompson-tomo thompson-tomo Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is still failing with this change. This is now the only issue as we can see the other workflows are now running as expected.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You're right—the earlier suggestion fixed the regex semantics but not the link check. .lycheeignore is itself passed to Lychee, so the entry is discovered as a URL and remains the only error in the current run.

Could we use https?:\/\/github\.com\/\$GITHUB_REPOSITORY\/actions\/runs\/\$GITHUB_RUN_ID instead? It still matches the workflow URL without appearing as a normal link.

Comment thread CHANGELOG.md Outdated
@thompson-tomo
thompson-tomo requested a review from MrAlias August 12, 2026 06:34
@thompson-tomo

Copy link
Copy Markdown
Author

All pieces of feedback have been worked through with ci adjusted to improve tooling update test changes.

The link checker is failing but that will be a pre-existing issue

@MrAlias MrAlias left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The earlier fail-fast and changelog blockers are addressed, but the monitoring and PR-validation paths are still coupled in ways that can disable or misroute validation.

Comment thread .github/workflows/markdown.yml Outdated
schedule:
# Everyday at 9:00 AM.
- cron: "0 9 * * *"
schedule:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

schedule is nested under workflow_dispatch, where only inputs is allowed. That makes the workflow invalid, so neither the daily run nor the new PR self-check can be registered.

Could we move schedule back to the same level as workflow_dispatch?

fix: false
globs: |
**/*.md
continue-on-error: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once the trigger indentation is fixed, the new pull_request path will reuse a job with issues: write, continue-on-error, and unconditional issue management. On same-repository PRs, a lint failure can create the shared workflow issue while the job stays green, and a passing PR can close an issue raised by main; on fork PRs, attempted mutations are denied.

Could we give PRs a read-only path where lint failures fail the job, and keep issue management only for non-PR monitoring runs?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have split the report step into it's own job with different path.

You don't want this job to fail the ci as it is scanning the entire repo. We have the fail fast check which only fails if a changed file fails the linting.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The split keeps issue writes out of PRs, but continue-on-error remains unconditional, so a failure in this self-check still leaves the PR workflow green. The fail-fast workflow only runs for Markdown changes, so it does not cover a PR that changes this workflow.

Could we make continue-on-error conditional on github.event_name == 'schedule'?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If i do that, the following will occur. lets say a new linter rule is introduced then the pr to update it will now fail rather than doing what it does now which is allowing it to pass and instead creating an action item to address it.

In effect it eliminate nearly any need for the reporting workflows as it would only occur if someone merged a pr with a failed linter check.

@MrAlias MrAlias left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The job split resolves the permission coupling, but the scheduled reporting path is not functional yet.

- name: Manage issue
if: github.event_name == 'schedule'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This job starts on a fresh runner without checkout or GH_REPO, so gh issue list exits before it can manage anything. The conditions below also use steps.markdownlint.outcome, but that step is in lint-markdown; here it is empty, which would close an existing issue after a failure and never create a missing one.

Could we pass the repository explicitly and read needs.lint-markdown.outputs.lint_outcome?

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