chore: Add Markdownlint cleanup job #16036

nschonni · 2022-05-15T01:06:55Z

GitHub Action to create daily PR fixes for auto-fixable Markdownlint issues

.github/workflows/markdown-lint-fix.yml

OnkarRuikar · 2022-05-15T05:35:59Z

.github/workflows/markdown-lint-fix.yml

+          branch: daily-markdownlint-fix
+          title: "Daily Markdownlint auto-cleanup"
+          body: |
+            Auto-fix cleanup PR using Markdown CLI.


Can we append unfixable issues to the body?
Like:

./files/en-us/web/index.md:2 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "### Title"]

Is it possible to pipe the output to a lint.log file and attach file contents to the PR body.

A good compromise would be to link to the run that triggered the PR: https://github.com/mdn/content/actions/runs/${GITHUB_RUN_ID}

nschonni · 2022-05-15T17:32:50Z

I've been cleaning these up for awhile, there were only ~40 changed files when I tested it on my repo nschonni#105

nschonni · 2022-05-15T18:34:52Z

#16075 are the only outstanding issues currently

OnkarRuikar · 2022-05-16T04:18:17Z

You are right. I accidentally ran it with default config and got ~700 files updated. 😬
As per the current https://github.com/mdn/content/blob/main/.markdownlint.json config all the files are clean.

Also the processing all the files didn't take much time. Looks like it keeps track of modified files and directories.
So, using the filter "**/*.md" is ok.

We need to include issues, in PR body, that the linter couldn't fix and need manual work.

$ logs=$(markdownlint "**/*.md" --fix)

# append the $logs to the PR body

nschonni · 2022-05-24T03:45:09Z

BTW, I have been looking at the "append log to body", but it hasn't been simple and I don't know if I'll be able to resolve it

LeoMcA · 2022-07-20T10:07:26Z

Once #12316 is merged, should this be updated to use the yarn fix:md command instead of markdownlint-cli directly?

nschonni · 2022-07-20T16:14:54Z

Yes, I think using the script would make sense. If the other one lands, I'll adjust this one, but I didn't want either to block the other

.github/workflows/markdown-lint-fix.yml

teoli2003

LGTM. I would like a dev review before merging this as I'm not an expert in these workflows, but we definitively would like to make a try with this one.

Josh-Cena · 2022-08-23T18:31:32Z

.github/workflows/markdown-lint-fix.yml

+          commit-message: "chore: auto-fix Markdownlint issues"
+          branch: daily-markdownlint-fix
+          title: "Daily Markdownlint auto-cleanup"
+          body: |
+            No unfixable issues found


Can we use @mdn-bot as committer + author instead?

The Author and/or committer can be overridden https://github.com/peter-evans/create-pull-request#action-inputs

Yeah, it's more of a question of "should" instead of "can" 😄

Yes, it would be nice to have a bot as the author, it makes clear that it is an automatic process.

The latest user who creates the schedule trigger is the author by default. [ref]

To make @mdn-bot the author see #16036 (comment)

I think it's okay to use @mdn-bot as author (though committer is not quite correct), but the default (GitHub <[email protected]>) wouldn't hurt either.

.github/workflows/markdown-lint-fix.yml

OnkarRuikar · 2022-08-24T08:14:19Z

This is an interesting workflow. I have a few queries about how this is going to work:

At what time will it trigger? Midnight UTC-8 Los Angeles time?
Do we want auto merge on this like dependabot PRs? Or we are going to merge manually?
In their example they've said:

When you merge the pull request make sure to choose the Rebase and merge option. This will make the fork's commits match the commits on the upstream.

Which option are we suppose to use for this one?
What should happen the next day if we fail to merge the PR within 24 hours? Create a brand new PR? or Append to the existing one?
Do we want to reuse the daily-markdownlint-fix branch everyday? Or Delete and create a brand new branch everyday?

teoli2003 · 2022-08-24T08:34:45Z

* At what time will it trigger? Midnight `UTC-8 Los Angeles` time?

This is not critical, but it looks like a good time. Night in the US, and about 10am Central European time.

* Do we want auto merge on this like dependabot PRs? Or we are going to merge manually?

I think we should start by merging them manually. If it goes well, we can later do what is done with the daily yari update and merge them automatically.

* In [their example](https://github.com/peter-evans/create-pull-request/blob/main/docs/examples.md) they've said:
  > When you merge the pull request make sure to choose the [`Rebase and merge`](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#rebase-and-merge-your-pull-request-commits) option. This will make the fork's commits match the commits on the upstream.

Squash and merge should be the default, like for any other PR on mdn/content.

  Which option are we suppose to use for this one?

* What should happen the next day if we fail to merge the PR within 24 hours? Create a brand new PR? or Append to the existing one?

I like what dependabot is doing: closing the old PR and opening a new one.

* Do we want to reuse the `daily-markdownlint-fix` branch everyday? Or Delete and create a brand new branch everyday?

I'm a bit afraid of reusing a branch: it could be in a strange state. What does dependabot does? It works pretty well and we should do the same.

.github/workflows/markdown-lint-fix.yml

caugner · 2022-08-24T10:02:08Z

.github/workflows/markdown-lint-fix.yml

+          commit-message: "chore: auto-fix Markdownlint issues"
+          branch: daily-markdownlint-fix
+          title: "Daily Markdownlint auto-cleanup"
+          body: |
+            No unfixable issues found


I think it's okay to use @mdn-bot as author (though committer is not quite correct), but the default (GitHub <[email protected]>) wouldn't hurt either.

caugner · 2022-08-24T10:04:56Z

What should happen the next day if we fail to merge the PR within 24 hours? Create a brand new PR? or Append to the existing one?

Quote from https://github.com/peter-evans/create-pull-request#action-behaviour:

If a pull request already exists it will be updated if necessary. Local changes in the Actions workspace, or changes on the base branch, can cause an update. If no update is required the action exits silently.

nschonni · 2022-08-24T18:17:51Z

I've added the MDN-bot as the committer rather than the author. That way the author of the PR will remain the bot if it's triggered by the cron job, but the user if it's manually triggered by a member to generate the PR ad-hoc

.github/workflows/markdown-lint-fix.yml

Josh-Cena

LGTM, we can play with this

teoli2003 · 2022-08-25T04:43:45Z

Yes, let's try this.

nschonni · 2022-08-29T14:04:58Z

@caugner you still have a blocking review on this

.github/workflows/markdown-lint-fix.yml

teoli2003 · 2022-09-05T15:54:49Z

Is anything blocking this? (I would prefer to merge this near the beginning of the week rather than close to the weekend)

Josh-Cena · 2022-09-05T15:56:00Z

None, just waiting on @caugner to re-review

yarn cache chore: use mdn-bot as author chore: update PR titles and branch

caugner

LGTM, just one last suggestion to link directly to the log of the action run.

.github/workflows/markdown-lint-fix.yml

Co-authored-by: Claas Augner <[email protected]>

* chore: Add Markdownlint cleanup job yarn cache chore: use mdn-bot as author chore: update PR titles and branch * chore: add direct link to failing job Co-authored-by: Claas Augner <[email protected]> Co-authored-by: Claas Augner <[email protected]>

nschonni requested review from a team and fiji-flo and removed request for a team May 15, 2022 01:06

OnkarRuikar reviewed May 15, 2022

View reviewed changes

OnkarRuikar mentioned this pull request May 17, 2022

Daily Markdownlint cleanup #16124

Merged

1 task

nschonni mentioned this pull request May 27, 2022

Add a Markdown linter for all .md files #10787

Closed

7 tasks

nschonni force-pushed the markdownlint-autofix branch 2 times, most recently from 9ca981a to 47abd2f Compare June 16, 2022 19:46

Josh-Cena added the infra Infrastructure issues (npm, GitHub Actions, linting) for this repo label Aug 13, 2022

nschonni mentioned this pull request Aug 17, 2022

chore: make Markdownlint a dependency and script #12316

Merged

nschonni force-pushed the markdownlint-autofix branch from 47abd2f to fccb97e Compare August 23, 2022 14:28

nschonni requested a review from a team as a code owner August 23, 2022 14:28

teoli2003 suggested changes Aug 23, 2022

View reviewed changes

.github/workflows/markdown-lint-fix.yml Outdated Show resolved Hide resolved

nschonni requested review from teoli2003 and removed request for fiji-flo August 23, 2022 14:57

teoli2003 approved these changes Aug 23, 2022

View reviewed changes

Josh-Cena reviewed Aug 23, 2022

View reviewed changes

OnkarRuikar reviewed Aug 24, 2022

View reviewed changes

.github/workflows/markdown-lint-fix.yml Outdated Show resolved Hide resolved

OnkarRuikar reviewed Aug 24, 2022

View reviewed changes

.github/workflows/markdown-lint-fix.yml Outdated Show resolved Hide resolved

caugner requested changes Aug 24, 2022

View reviewed changes

nschonni force-pushed the markdownlint-autofix branch from bc43c2f to 2aa6f3d Compare August 24, 2022 16:31

nschonni requested a review from caugner August 24, 2022 18:45

Josh-Cena reviewed Aug 24, 2022

View reviewed changes

.github/workflows/markdown-lint-fix.yml Outdated Show resolved Hide resolved

Josh-Cena approved these changes Aug 24, 2022

View reviewed changes

Josh-Cena mentioned this pull request Aug 25, 2022

Daily markdownlint #19953

Merged

nschonni force-pushed the markdownlint-autofix branch 2 times, most recently from ca64f21 to c981724 Compare August 26, 2022 22:02

nschonni commented Aug 31, 2022

View reviewed changes

.github/workflows/markdown-lint-fix.yml Outdated Show resolved Hide resolved

chore: Add Markdownlint cleanup job

b39f11f

yarn cache chore: use mdn-bot as author chore: update PR titles and branch

nschonni force-pushed the markdownlint-autofix branch from c981724 to b39f11f Compare September 5, 2022 16:32

caugner approved these changes Sep 6, 2022

View reviewed changes

.github/workflows/markdown-lint-fix.yml Outdated Show resolved Hide resolved

chore: add direct link to failing job

21b95db

Co-authored-by: Claas Augner <[email protected]>

caugner approved these changes Sep 6, 2022

View reviewed changes

nschonni mentioned this pull request Sep 7, 2022

Daily markdownlint #20369

Merged

teoli2003 merged commit ada64d8 into mdn:main Sep 7, 2022

nschonni deleted the markdownlint-autofix branch September 7, 2022 03:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Add Markdownlint cleanup job #16036

chore: Add Markdownlint cleanup job #16036

nschonni commented May 15, 2022

OnkarRuikar May 15, 2022

caugner Sep 6, 2022

nschonni commented May 15, 2022

nschonni commented May 15, 2022

OnkarRuikar commented May 16, 2022

nschonni commented May 24, 2022

LeoMcA commented Jul 20, 2022 •

edited

Loading

nschonni commented Jul 20, 2022

teoli2003 left a comment

Josh-Cena Aug 23, 2022 •

edited

Loading

nschonni Aug 23, 2022

Josh-Cena Aug 23, 2022

teoli2003 Aug 24, 2022

OnkarRuikar Aug 24, 2022

caugner Aug 24, 2022

OnkarRuikar commented Aug 24, 2022 •

edited

Loading

teoli2003 commented Aug 24, 2022 •

edited

Loading

caugner Aug 24, 2022

caugner commented Aug 24, 2022

nschonni commented Aug 24, 2022

Josh-Cena left a comment

teoli2003 commented Aug 25, 2022

nschonni commented Aug 29, 2022

teoli2003 commented Sep 5, 2022

Josh-Cena commented Sep 5, 2022

caugner left a comment

chore: Add Markdownlint cleanup job #16036

chore: Add Markdownlint cleanup job #16036

Conversation

nschonni commented May 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nschonni commented May 15, 2022

nschonni commented May 15, 2022

OnkarRuikar commented May 16, 2022

nschonni commented May 24, 2022

LeoMcA commented Jul 20, 2022 • edited Loading

nschonni commented Jul 20, 2022

teoli2003 left a comment

Choose a reason for hiding this comment

Josh-Cena Aug 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OnkarRuikar commented Aug 24, 2022 • edited Loading

teoli2003 commented Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

caugner commented Aug 24, 2022

nschonni commented Aug 24, 2022

Josh-Cena left a comment

Choose a reason for hiding this comment

teoli2003 commented Aug 25, 2022

nschonni commented Aug 29, 2022

teoli2003 commented Sep 5, 2022

Josh-Cena commented Sep 5, 2022

caugner left a comment

Choose a reason for hiding this comment

LeoMcA commented Jul 20, 2022 •

edited

Loading

Josh-Cena Aug 23, 2022 •

edited

Loading

OnkarRuikar commented Aug 24, 2022 •

edited

Loading

teoli2003 commented Aug 24, 2022 •

edited

Loading