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

build: added doc linting when runnning make lint #18472

Closed
wants to merge 1 commit into from

Conversation

camilo86
Copy link
Contributor

Fixes: #18466

Lint docs when running make lint. Added a call to lint-md inside the lint command in the Makefiile

Checklist
Affected core subsystem(s)

build

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jan 31, 2018
@danbev
Copy link
Contributor

danbev commented Jan 31, 2018

@camilo86 Thanks for this! I think this PR should be opened against the master branch first though, and then it can be cherry-picked to the v9.5.0-proposal branch (unless I'm missing some context here and this should be handled differently, in that case just ignore me).

@joyeecheung
Copy link
Member

joyeecheung commented Jan 31, 2018

@danbev @camilo86 Yes, please retarget this PR to the master branch. FWIW, this is how:

  1. Fetch the latest master to your directory via something like git checkout master -> git fetch upstream master -> git reset --hard master (I am assuming upstream is the remote pointing to this repo)

  2. Regenerate this patch from the master branch. Note that you can't rebase this branch onto master since v9.x is very different from master now. You can either

    1. Make another branch from master and cherry-pick this commit
    git checkout master
    git checkout -b new-pr-branch-name
    git cherry-pick b2e4d19
    git branch -D v9.5.0-proposal
    git branch -m v9.5.0-proposal
    

    (BTW v9.5.0-proposal is not a good PR branch name since there is a duplicate in the upstream...but you can still finish this PR with it. We usually use a name that describes the patch though)

    1. or reset your current PR branch to master then edit Makefile since it's just a one-line change.
    git checkout v9.5.0-proposal  # current PR branch
    git reset --hard master
    git cherry-pick b2e4d19  # or you can just edit the file but I think git would remember this sha if you have not prune the repo
    
  3. Push the "Edit" button next to the PR title on this page, set the base branch to master

  4. git push --force from your local edited PR branch to the remote PR branch.

@camilo86 camilo86 changed the base branch from v9.5.0-proposal to master January 31, 2018 13:41
@camilo86
Copy link
Contributor Author

Thanks @danbev @joyeecheung . I changed the PR branch to master. Let me know if there is anything else that needs change.

@joyeecheung joyeecheung self-assigned this Feb 1, 2018
@joyeecheung
Copy link
Member

@BridgeAR
Copy link
Member

BridgeAR commented Feb 1, 2018

I am not certain but I thought there was a reason it was not done anymore?

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 1, 2018
@joyeecheung
Copy link
Member

joyeecheung commented Feb 1, 2018

@BridgeAR I guess it might be because we didn't want make lint fail when remark-cli was not there? But that should be fixed by #16893

@BridgeAR
Copy link
Member

BridgeAR commented Feb 1, 2018

Makes sense. Thanks for the heads up.

@targos
Copy link
Member

targos commented Feb 6, 2018

@targos
Copy link
Member

targos commented Feb 6, 2018

Landed in 0548034. Thanks!

@targos targos closed this Feb 6, 2018
targos pushed a commit that referenced this pull request Feb 6, 2018
Fixes: #18466

PR-URL: #18472
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos targos removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 6, 2018
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Fixes: #18466

PR-URL: #18472
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Fixes: #18466

PR-URL: #18472
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Fixes: #18466

PR-URL: #18472
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 21, 2018
gibfahn pushed a commit that referenced this pull request Apr 13, 2018
Fixes: #18466

PR-URL: #18472
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 2, 2018
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Fixes: nodejs#18466

PR-URL: nodejs#18472
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants