src: use executable + pid as inspector context id #17087
Conversation
nodejs-github-bot
added
c++
|
is this the context id that I'm looking for in #16591? |
|
@targos Probably not. This PR just changes the context's human-readable name in the UI. |
bnoordhuis
force-pushed
the
better-inspector-context-name
branch
from
227308e
to
c720147
Compare
|
Minor tweak for Windows: https://ci.nodejs.org/job/node-test-pull-request/11559/ @TimothyGu Yep, I'll include that if I can fit it in <= 50 columns. (Don't say anything about relaxing that rule, I can't hear you with my hands over my ears.) edit: again with smartos fixup: https://ci.nodejs.org/job/node-test-pull-request/11563/ |
There are a few places where we paper over the fact that getpid() is called GetCurrentProcessId() on Windows. Let's move it into a function. PR-URL: nodejs#17087 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Report (for example) "node[1337]" as the human-readable name rather than the more generic and less helpful "Node.js Main Context." While not perfect yet, it should be an improvement to people that debug multiple processes from DevTools, VS Code, etc. PR-URL: nodejs#17087 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
bnoordhuis
force-pushed
the
better-inspector-context-name
branch
from
ea4f277
to
f526deb
Compare
There are a few places where we paper over the fact that getpid() is called GetCurrentProcessId() on Windows. Let's move it into a function. PR-URL: #17087 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Report (for example) "node[1337]" as the human-readable name rather than the more generic and less helpful "Node.js Main Context." While not perfect yet, it should be an improvement to people that debug multiple processes from DevTools, VS Code, etc. PR-URL: #17087 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
|
This does not land cleany on v6.x, should we manually backport? As an aside, when landing PRs with multiple commits can you please add a comment specifying which commits landed. It is hard to tell from the merge info which commits to include |
There are a few places where we paper over the fact that getpid() is called GetCurrentProcessId() on Windows. Let's move it into a function. PR-URL: #17087 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Report (for example) "node[1337]" as the human-readable name rather than the more generic and less helpful "Node.js Main Context." While not perfect yet, it should be an improvement to people that debug multiple processes from DevTools, VS Code, etc. PR-URL: #17087 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
No, the relevant inspector API doesn't exist in v6.x. I'll update the labels.
What merge info are you referring to? I thought you were using the GH API to back-port pull requests. You can get the parent commit from the base/sha field in https://github.com/gitapi/repos/nodejs/node/pulls/17087. |
There are a few places where we paper over the fact that getpid() is called GetCurrentProcessId() on Windows. Let's move it into a function. PR-URL: #17087 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Report (for example) "node[1337]" as the human-readable name rather than the more generic and less helpful "Node.js Main Context." While not perfect yet, it should be an improvement to people that debug multiple processes from DevTools, VS Code, etc. PR-URL: #17087 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
We use the GH API to generate the list of pull requests to triage, we backport with Is the easiest to copy-paste from. The Collaborator guide requires it when you merge multiple commits, or when you close without merging:
https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#technical-howto Basically if you wanted to make backporters' lives easier, you'd always put |
Then can I suggest you start using the base/sha field to get the merge base? Less margin for error and it's less labor all around. We're programmers, we automate all the things, right? |
Report (for example) "node[1337]" as the human-readable name rather
than the more generic and less helpful "Node.js Main Context."
While not perfect yet, it should be an improvement to people that
debug multiple processes from DevTools, VS Code, etc.
CI: https://ci.nodejs.org/job/node-test-pull-request/11489/