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: correctly use .markdownlintignore in Makefile #16060

Merged
merged 1 commit into from Jun 27, 2022

Conversation

bmish
Copy link
Sponsor Member

@bmish bmish commented Jun 27, 2022

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

  • Other, please explain:

Improve robustness of internal npm run lint command.

What changes did you make? (Give an overview)

In the Makefile, we run markdownlint on all of our documentation files.

We previously had a naive check to ignore files that are mentioned in .markdownlintignore. But this didn't handle nested node_modules which will be created after running npm install in folders like docs/.

In this PR, I have updated the check to use the existing ignore library to correctly filter out the gitignore-style ignore patterns in .markdownlintignore.

Note that the reason we have to manually do this ignore-filtering is because markdownlint does not support .markdownlintignore yet whereas markdownlint-cli and editor extensions do. In the Makefile, we have to use markdownlint because it has a Node API whereas markdownlint-cli doesn't.

Thank you to @amareshsm for finding this issue.

Is there anything you'd like reviewers to focus on?

@eslint-github-bot eslint-github-bot bot added triage An ESLint team member will look at this issue soon chore This change is not user-facing labels Jun 27, 2022
@netlify
Copy link

@netlify netlify bot commented Jun 27, 2022

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 516b1d7
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/62b8f4278a9c860008eb1635
😎 Deploy Preview https://deploy-preview-16060--docs-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jun 27, 2022
@mdjermanovic
Copy link
Member

@mdjermanovic mdjermanovic commented Jun 27, 2022

Makefile, we have to use markdownlint because it has a Node API whereas markdownlint-cli doesn't.

I'm not sure why we need to use Node API. We could maybe use markdownlint-cli the same way we use eslint, nyc, karma and other CLIs in Makefile.js.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

LGTM, thanks!

This correctly handles .markdownlintignore file and fixes the problem we currently have ( npm test and other commands that run markdownlint fail if docs/node_modules was installed).

@mdjermanovic mdjermanovic merged commit 6840940 into eslint:main Jun 27, 2022
20 checks passed
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this issue Jul 4, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.18.0` -> `8.19.0`](https://renovatebot.com/diffs/npm/eslint/8.18.0/8.19.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.19.0`](https://github.com/eslint/eslint/releases/tag/v8.19.0)

[Compare Source](eslint/eslint@v8.18.0...v8.19.0)

#### Features

-   [`7023628`](eslint/eslint@7023628) feat: add importNames support for patterns in no-restricted-imports ([#&#8203;16059](eslint/eslint#16059)) (Brandon Scott)
-   [`472c368`](eslint/eslint@472c368) feat: fix handling of blockless `with` statements in indent rule ([#&#8203;16068](eslint/eslint#16068)) (Milos Djermanovic)

#### Bug Fixes

-   [`fc81848`](eslint/eslint@fc81848) fix: throw helpful exception when rule has wrong return type ([#&#8203;16075](eslint/eslint#16075)) (Bryan Mishkin)

#### Documentation

-   [`3ae0574`](eslint/eslint@3ae0574) docs: Remove duplicate rule descriptions ([#&#8203;16052](eslint/eslint#16052)) (Amaresh  S M)
-   [`f50cf43`](eslint/eslint@f50cf43) docs: Add base href to each page to fix relative URLs ([#&#8203;16046](eslint/eslint#16046)) (Nicholas C. Zakas)
-   [`ae4b449`](eslint/eslint@ae4b449) docs: make logo link clickable on small width screens ([#&#8203;16058](eslint/eslint#16058)) (Milos Djermanovic)
-   [`280f898`](eslint/eslint@280f898) docs: use only fenced code blocks ([#&#8203;16044](eslint/eslint#16044)) (Milos Djermanovic)
-   [`f5d63b9`](eslint/eslint@f5d63b9) docs: add listener only if element exists ([#&#8203;16045](eslint/eslint#16045)) (Amaresh  S M)
-   [`8b639cc`](eslint/eslint@8b639cc) docs: add missing migrating-to-8.0.0 in the user guide ([#&#8203;16048](eslint/eslint#16048)) (唯然)
-   [`b8e68c1`](eslint/eslint@b8e68c1) docs: Update release process ([#&#8203;16036](eslint/eslint#16036)) (Nicholas C. Zakas)
-   [`6d0cb11`](eslint/eslint@6d0cb11) docs: remove table of contents from markdown text ([#&#8203;15999](eslint/eslint#15999)) (Nitin Kumar)

#### Chores

-   [`e884933`](eslint/eslint@e884933) chore: use `github-slugger` for markdown anchors ([#&#8203;16067](eslint/eslint#16067)) (Strek)
-   [`02e9cb0`](eslint/eslint@02e9cb0) chore: revamp carbon ad style ([#&#8203;16078](eslint/eslint#16078)) (Amaresh  S M)
-   [`b6aee95`](eslint/eslint@b6aee95) chore: remove unwanted comments from rules markdown ([#&#8203;16054](eslint/eslint#16054)) (Strek)
-   [`6840940`](eslint/eslint@6840940) chore: correctly use .markdownlintignore in Makefile ([#&#8203;16060](eslint/eslint#16060)) (Bryan Mishkin)
-   [`48904fb`](eslint/eslint@48904fb) chore: add missing images ([#&#8203;16017](eslint/eslint#16017)) (Amaresh  S M)
-   [`910f741`](eslint/eslint@910f741) chore: add architecture to nav ([#&#8203;16039](eslint/eslint#16039)) (Strek)
-   [`9bb24c1`](eslint/eslint@9bb24c1) chore: add correct incorrect in all rules doc ([#&#8203;16021](eslint/eslint#16021)) (Deepshika S)
-   [`5a96af8`](eslint/eslint@5a96af8) chore: prepare versions data file ([#&#8203;16035](eslint/eslint#16035)) (Nicholas C. Zakas)
-   [`50afe6f`](eslint/eslint@50afe6f) chore: Included githubactions in the dependabot config ([#&#8203;15985](eslint/eslint#15985)) (Naveen)
-   [`473411e`](eslint/eslint@473411e) chore: add deploy workflow for playground ([#&#8203;16034](eslint/eslint#16034)) (Milos Djermanovic)
-   [`a30b66c`](eslint/eslint@a30b66c) chore: fix print style ([#&#8203;16025](eslint/eslint#16025)) (Amaresh  S M)
-   [`f4dad59`](eslint/eslint@f4dad59) chore: add noindex meta tag ([#&#8203;16016](eslint/eslint#16016)) (Milos Djermanovic)
-   [`db387a8`](eslint/eslint@db387a8) chore: fix sitemap ([#&#8203;16026](eslint/eslint#16026)) (Milos Djermanovic)
-   [`285fbc5`](eslint/eslint@285fbc5) chore: remove TOC from printable ([#&#8203;16020](eslint/eslint#16020)) (Strek)
-   [`8e84c21`](eslint/eslint@8e84c21) chore: remove ligatures from fonts ([#&#8203;16019](eslint/eslint#16019)) (Strek)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

 **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1444
Reviewed-by: 6543 <6543@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion chore This change is not user-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants