FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: lint deprecation codes by aduh95 · Pull Request #33433 · nodejs/node · GitHub

/ node Public

doc: lint deprecation codes - #33433

Closed
aduh95 wants to merge 2 commits into
nodejs:masterfrom
aduh95:lint-deprecation-codes
Closed

doc: lint deprecation codes#33433
aduh95 wants to merge 2 commits into
nodejs:masterfrom
aduh95:lint-deprecation-codes

Conversation

aduh95 commented May 16, 2020
edited
Loading

Copy link
Copy Markdown
Contributor

Add a rule to make sure deprecation codes are in order. This is to discourage the use of placeholders by contributors when creating new deprecations, and to relieve authors the responsibility to swap them with the appropriate code on landing.

Suggested by @targos in #33430 (comment).

Blocked by #33430.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels May 16, 2020
Comment thread doc/api/deprecations.md Outdated

Copy link
Copy Markdown
Member

Is it possible to implement this as a lint rule as opposed to a new tool and test?

aduh95 commented May 16, 2020

Copy link
Copy Markdown
Contributor Author

Is it possible to implement this as a lint rule as opposed to a new tool and test?

@richardlau Do you have an example I can follow on how to implement that? I'm not familiar with markdown linters (yet 😅).

richardlau commented May 16, 2020
edited
Loading

Copy link
Copy Markdown
Member

https://github.com/nodejs/node/blob/master/tools/eslint-rules/documented-errors.js, for example.

Ah wait that is a js lint rule. I think the custom markdown lint rules are external to this repo. Cc @Trott

Copy link
Copy Markdown
Member

@richardlau we could lint that all deprecation codes adhere to a specific format in lib.

Add a rule to make sure deprecation codes are in order.
aduh95 force-pushed the lint-deprecation-codes branch from 288f0e1 to ca5f40f Compare May 18, 2020 08:21
aduh95 marked this pull request as ready for review May 18, 2020 08:23

aduh95 commented May 29, 2020

Copy link
Copy Markdown
Contributor Author

@richardlau we could lint that all deprecation codes adhere to a specific format in lib.

That would cover runtime deprecation only right?

Copy link
Copy Markdown
Collaborator

Trott commented Jul 4, 2020

Copy link
Copy Markdown
Member

Ah wait that is a js lint rule. I think the custom markdown lint rules are external to this repo. Cc @Trott

They are in https://github.com/nodejs/remark-preset-lint-node.

aduh95 commented Oct 16, 2020

Copy link
Copy Markdown
Contributor Author

Do we still want to do that? Every now and then a new PR appears to replace DEPXXX placeholders that made their way to master, but on the other hand the tooling is supposed to take care of it now…

I personally don't get why we use a placeholder, because if a new deprecation lands before the one on the PR, the author still has to fix the git conflict, so the placeholder doesn't help there. If someone thinks it's still a valid idea, I'm willing to work on it, otherwise let's close this.

aduh95 added the stalled Issues and PRs that are stalled. label Oct 16, 2020

Copy link
Copy Markdown
Contributor

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

Trott commented Oct 17, 2020

Copy link
Copy Markdown
Member

I personally don't get why we use a placeholder, because if a new deprecation lands before the one on the PR, the author still has to fix the git conflict, so the placeholder doesn't help there. If someone thinks it's still a valid idea, I'm willing to work on it, otherwise let's close this.

In theory at least, one thing the placeholder gets us is prevention of duplicate DEP codes landing and nobody noticing.

aduh95 closed this Nov 14, 2020
aduh95 deleted the lint-deprecation-codes branch November 14, 2020 11:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Issues and PRs related to the documentations. stalled Issues and PRs that are stalled. tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL