-
Notifications
You must be signed in to change notification settings - Fork 780
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
[repo] markdownlint ci changes #5281
[repo] markdownlint ci changes #5281
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #5281 +/- ##
==========================================
- Coverage 83.38% 83.07% -0.31%
==========================================
Files 297 272 -25
Lines 12531 11965 -566
==========================================
- Hits 10449 9940 -509
+ Misses 2082 2025 -57
Flags with carried forward coverage won't be shown. Click here to find out more.
|
- name: run markdownlint | ||
run: markdownlint . | ||
uses: DavidAnson/[email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a general question - how do we decide when to trust something like this or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd hope that this will be more secure. I assume that now that we're using an actual version rather than just blindly downloading the latest, that dependabot can help alert us of vulnerabilities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a great question and I don't have an answer other than it is something we should discuss on SIG 🤣
Some thoughts...
-
This particular thing
DavidAnson/markdownlint-cli2-action
seems to be made by the same guy who makesmarkdownlint
itself so high confidence here. -
There is table which shows permissions for "pull requests from public forked repositories": https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token
I don't worry much about the CI workflow(s) because it seems GitHub has a pretty good safe-by-default policy. I would worry more about a release job using random actions but for this repo we don't have a release job (we do it manually). The contrib repo does use workflows to release though.
Changes
Resolving build failures due to links in markdown files (ex https://github.com/open-telemetry/opentelemetry-dotnet/actions/runs/7701990497/job/20993415110?pr=5255#step:4:14) by switching to markdownlint-cli2 GitHub Action v14 (15 is the latest and generates errors)
Also updated the CI pipeline so that md & dotnet-format branches are run if build artifacts (like workflows) are changed
Merge requirement checklist