JS Packages: update URL structure#41101
Conversation
|
@tbradsha yup. done! https://docs.npmjs.com/cli/v11/configuring-npm/package-json#repository It looks like it does expect |
I was just reading that doc and was going to mention that but checked...apparently we already use |
anomiex
left a comment
There was a problem hiding this comment.
I think npm may be being overly strict here.
The URL should be a publicly available (perhaps read-only) URL that can be handed directly to a VCS program without any modification. It should not be a URL to an html project page that you put in your browser. It's for computers.
git clone https://github.com/Automattic/jetpack appears to work fine.
For that matter, git clone git+https://github.com/Automattic/jetpack.git doesn't work. 🤷
$ git clone git+https://github.com/Automattic/jetpack.git
Cloning into 'jetpack'...
git: 'remote-git+https' is not a git command. See 'git --help'.
fatal: remote helper 'git+https' aborted session
| fi | ||
| URL="$(jq -r '.url' <<<"$JSON")" | ||
| if [[ "$URL" != "https://github.com/Automattic/jetpack.git" && "$URL" != "https://github.com/Automattic/jetpack" ]]; then | ||
| if [[ "$URL" != "git+https://github.com/Automattic/jetpack.git" && "$URL" != "https://github.com/Automattic/jetpack" ]]; then |
There was a problem hiding this comment.
Possibly the second one should also be changed?
There was a problem hiding this comment.
Or maybe remove it entirely, if we really want to match what npm pkg fix does.
There was a problem hiding this comment.
I was assuming .git suffix would mean git-consumable, whereas without it would be a normal user-friendly URL. But now that you quoted that line and showed that git clone doesn't work I'm even more confused.
I was trying to track down if/when this changed, but couldn't. I found the example in the documentation changed here:
npm/cli#7615
I did find there's a noGitPlus option on hosted-git-info, which seems to be what provides the normalized URL info:
https://github.com/npm/hosted-git-info/
There was a problem hiding this comment.
It seems to be something internal where git+https or git+ssh but, in either case, it knows it is a github link, so it doesn't look like it actually matters?
I could see where an enterprise version of GH or other self-hosted git might have more of a need for it.
I'm fine dropping this as not needed since npm is figuring it out just fine it seems like. It doesn't really seem to be adding value unless I'm missing something.
Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
|
I am going to close this in the "not broken, don't fix it" category. NPM fixes it on the fly and, as pointed out, it seems inconsistent with their docs on the functionality. |
When monitoring a npm package autopublish, I noticed this in the logs:
Proposed changes:
npm pkg fixin js-packages that are auto-published to resolve.Other information:
Jetpack product discussion
none
Does this pull request change what data or activity we track or use?
no
Testing instructions: