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.

Sign up for GitHub

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

Jump to bottom

doc: add link to VSCode config in testing #24012

Closed
wants to merge 1 commit into from
Closed

doc: add link to VSCode config in testing #24012

wants to merge 1 commit into from

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Nov 1, 2018

Refs: https://github.com/orgs/nodejs/teams/collaborators/discussions/58

This config will help contributors to run/debug tests on VSCode

Checklist

@nodejs-github-bot
Copy link
Collaborator

@trivikr build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1447/pipeline

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Nov 1, 2018
Trott
Trott previously requested changes Nov 1, 2018
View reviewed changes
Copy link
Member

@Trott Trott left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That config repo was transferred into the org without opening an issue in the admin repo to discuss. (That process is not easy to surface if you don't know where it is, so I understand why it was missed!)

The config repo lacks things that might have been insisted upon during that process. For example, there is no Code of Conduct and no LICENSE file. 😱 I don't know if transferring it out is the answer and then opening the issue in the admin repo? @nodejs/tsc @nodejs/community-committee

I'm not sure we want to add IDE-specific configurations into BUILDING.md. (I'm also not sure we don't. Curious what others think.)

Trott
Trott reviewed Nov 1, 2018
View reviewed changes
BUILDING.md Outdated Show resolved Hide resolved
@Trott Trott dismissed their stale review November 1, 2018 06:04

Not actually going to block this...

@trivikr
Copy link
Member Author

trivikr commented Nov 1, 2018

Updates about node-vscode-config:

jasnell
jasnell requested changes Nov 1, 2018
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I love VSCode and use it daily, I'd much prefer moving this kind of stuff into a separate guide that can detail the use of multiple tools.

@refack
Copy link
Contributor

refack commented Nov 1, 2018 •
edited
Loading

I'd much prefer moving this kind of stuff into a separate guide that can detail the use of multiple tools.

👍

I'm not sure we want to add IDE-specific configurations into BUILDING.md.

Personally I'd like to see the guide shrink... I feel that as the top level guide it's too verbose. So I'm -1 on adding more specific cases to it, including IDE specific instructions, unless that can help to un-verbose the guide.

That said, IMHO this would be useful to have in a separate guide, referred to from the main guide.

@trivikr trivikr mentioned this pull request Nov 1, 2018
Merged
@trivikr
Copy link
Member Author

trivikr commented Nov 1, 2018

@jasnell Steps taken:

@trivikr
Copy link
Member Author

trivikr commented Nov 6, 2018

Since I ran out of time, I forgot to show the VSCode launch configs being used in my NodeConfEU talk today.

Can we push this code so that new contributors don't get stuck finding out the VSCode launch config while contributing to Node?
On Google, searching for node VSCode Config shows which VSCode Config to use while debugging Node.js, and not Node.js core.

@Trott
Copy link
Member

Trott commented Nov 7, 2018

At the risk of getting too picky here, should we refrain from having developer stuff in the build doc and instead put that in CONTRIBUTING.md or something else that is aimed at developers working on Node.js core? BUILDING.md has a broader audience than that IMO.

@Trott
Copy link
Member

Trott commented Nov 7, 2018

(Granted, there are probably things in BUILDING.md already that, based on that criteria, should be moved out, like coverage generation.)

@trivikr trivikr added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Nov 9, 2018
@trivikr
Copy link
Member Author

trivikr commented Nov 9, 2018

Added tsc-agenda label, for discussions related to following questions:

  • What should be the name for new repo which stores editor configs?
  • Where should we link the repo from Node.js core? Should it link from BUILDING.md or CONTRIBUTING.md or somewhere else?

Follow up required on PRs:

@mhdawson mhdawson mentioned this pull request Nov 12, 2018
Closed
@mhdawson mhdawson mentioned this pull request Nov 19, 2018
Closed
@trivikr trivikr removed the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Nov 20, 2018
@trivikr
Copy link
Member Author

trivikr commented Nov 20, 2018

Removed tsc-agenda label as we agreed on name node-code-ide-configs in nodejs/TSC#633

@trivikr trivikr force-pushed the add-vscode-config branch 2 times, most recently from 7c5dc9a to fcd82e6 Compare October 24, 2019 02:52
@trivikr
Copy link
Member Author

trivikr commented Oct 24, 2019

@Trott @jasnell This PR is ready for review.

trivikr added a commit that referenced this pull request Nov 11, 2019
@trivikr
doc: add link to node-code-ide-configs in testing
e55d653 
Refs: https://github.com/orgs/nodejs/teams/collaborators/discussions/58

This config will help contributors to run/debug tests on VSCode

PR-URL: #24012
Reviewed-By: James M Snell <jasnell@gmail.com>
@trivikr
Copy link
Member Author

trivikr commented Nov 11, 2019

Landed in e55d653

@trivikr trivikr closed this Nov 11, 2019
@trivikr trivikr deleted the add-vscode-config branch November 11, 2019 21:57
MylesBorins pushed a commit that referenced this pull request Nov 17, 2019
@trivikr @MylesBorins
doc: add link to node-code-ide-configs in testing
1133981 
Refs: https://github.com/orgs/nodejs/teams/collaborators/discussions/58

This config will help contributors to run/debug tests on VSCode

PR-URL: #24012
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Nov 19, 2019
Merged
targos pushed a commit that referenced this pull request Dec 1, 2019
@trivikr @targos
doc: add link to node-code-ide-configs in testing
60a0f1e 
Refs: https://github.com/orgs/nodejs/teams/collaborators/discussions/58

This config will help contributors to run/debug tests on VSCode

PR-URL: #24012
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Dec 9, 2019
Closed
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
@trivikr @MylesBorins
doc: add link to node-code-ide-configs in testing
3710068 
Refs: https://github.com/orgs/nodejs/teams/collaborators/discussions/58

This config will help contributors to run/debug tests on VSCode

PR-URL: #24012
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Dec 23, 2019
Merged
@ronag ronag mentioned this pull request Apr 10, 2020
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott Trott left review comments

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@trivikr @nodejs-github-bot @refack @Trott @jasnell