Description

jakecastelli

opened on Apr 7, 2025

Issue body actions

I am on a new laptop and forgot some configs in my project specific config and failed the cq due to column length > 72 characters, therefore decided to land the PR manual by using core-util.

Followed the steps to:

pre: installed @node-core/utils through npm

1. create a scoped token
2. put token into ~/.ncurc
3. run git node land 57732
4. followed the prompt to setup remote-url and branch main to main

➜  node git:(add-lower-bound-range-test) ✗ ncu-config list
branch: main
upstream: remote-upstream

and then I encountered the following error message:

    at runSync (file:///Users/jake/.nvm/versions/node/v23.11.0/lib/node_modules/@node-core/utils/lib/run.js:109:11)
    at new Session (file:///Users/jake/.nvm/versions/node/v23.11.0/lib/node_modules/@node-core/utils/lib/session.js:30:28)
    at new LandingSession (file:///Users/jake/.nvm/versions/node/v23.11.0/lib/node_modules/@node-core/utils/lib/landing_session.js:24:5)
    at main (file:///Users/jake/.nvm/versions/node/v23.11.0/lib/node_modules/@node-core/utils/components/git/land.js:171:17)

I think it is trying to find remote-upstream as after I did git remote add remote-upstream https://github.com/nodejs/node.git the issue was resolved. However, this confused me for a few minutes, shouldn't it default to just upstream or attempt to add remote-upstream for the user automatically?

Activity

1. jakecastelli

   changed the title [-]git node land <PR number> failed without meaningful error message[/-] [+]setting up remote-upstream through ncu[/+] on Apr 7, 2025
2. aduh95 commented on Apr 7, 2025

   

   aduh95

   on Apr 7, 2025

   Contributor

   More actions

   It should certainly not add a remote without telling the user, if we're dealing with a security PR, it would be a breach of confidentiality to default to the public repo. Why did you ask it to use remote-upstream and not upstream if that's the name of your remote? Maybe that's where the docs needs improvements
3. jakecastelli commented on Apr 7, 2025

   

   jakecastelli

   on Apr 7, 2025

   MemberAuthor

   More actions

   It should certainly not add a remote without telling the user, if we're dealing with a security PR, it would be a breach of confidentiality to default to the public repo.

   Ah, I see, TIL 👍

   Why did you ask it to use remote-upstream and not upstream if that's the name of your remote? Maybe that's where the docs needs improvements

   I didn't think I ask it to use remote-upstream though, I just followed the default prompt from the ncu-config, I may have missed something, I will try again later.
4. aduh95 commented on Apr 7, 2025

   

   aduh95

   on Apr 7, 2025

   Contributor

   More actions

   node-core-utils/lib/session.js

   Lines 381 to 388 in dfa9c92

   	cli.warn('You have not told git-node the remote you want to sync with.');
   	cli.separator();
   	cli.info(
   	'For example, if your remote pointing to nodejs/node is' +
   	' `remote-upstream`, you can run:\n\n' +
   	' $ ncu-config set upstream remote-upstream');
   	cli.separator();
   	cli.setExitCode(1);

   The default prompt would suggest remote-upstream indeed, so maybe it'd worth suggesting upstream instead

   Reacted by jakecastelli

Issue actions