-
Notifications
You must be signed in to change notification settings - Fork 60.5k
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
Simplify repository element in npm's project.json to use https URL #1127
Conversation
"repository" : { | ||
"type" : "git", | ||
"url": "ssh://git@{% if currentVersion == "free-pro-team@latest" %}github.com{% else %}<em>HOSTNAME</em>{% endif %}/<em>OWNER</em>/<em>REPOSITORY</em>.git", | ||
"directory": "packages/name" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It explains the "directory" value in the previous paragraph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I don't think we need to document the directory
value here (it isn't used by GitHub Packages).
This is different than how
Ideally maybe we should say to use |
...github-packages-with-your-projects-ecosystem/configuring-npm-for-use-with-github-packages.md
Outdated
Show resolved
Hide resolved
…ystem/configuring-npm-for-use-with-github-packages.md
Interesting, I didn't know that. I tend to I guess If I'm adding it by hand, I much prefer the |
👋 @jcansdale @toddself, let me know when this is ready for a writer review! |
Thinking more about this. If a user does I think the following section: ... is most relevant for users who are adding the What do you think? |
@jcansdale i think that makes total sense! |
@janiceilene I think we're ready for a writer review. 👍 |
This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit. |
Hi @janiceilene, I've just marked this as not-stale. It's ready for a writer review. 🙂 |
This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit. |
We will review this as soon as we can! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good!
Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. |
Co-authored-by: Lana Brindley <github@lanabrindley.com>
Why:
The
repository
element in thepackage.json
file can be dramatically simplified from:..to just:
This will be less error prone because it can be easily copy/pasted from GitHub repository URLs.
We could simplify it further to just
"repository" : "OWNER/REPOSITORY",
, because npm treats GitHub as it's default Git repository host. This would break symmetry with GHES, so maybe thehttps
form is preferable?See here for
"repository" : "OWNER/REPOSITORY",
form:https://docs.npmjs.com/cli/v6/configuring-npm/package-json#repository:~:text=%22repository%22%3A%20%22npm%2Fnpm%22
What's being changed:
Check off the following: