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.

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

docs: Spell out "Install from Git repository" options properly #578

Closed

Conversation

MichalBryxi
Copy link

  • Create sections to make visual/logical distinction of individual parts.
  • Spell out all major possible syntaxes.
  • Add more explanation to individual parts of the URL like semver: and path:.
  • Copy & paste examples directly from tests so that we're reasonably sure they are correct.
  • Explain defaults.
  • Explain common cases like monorepo & forks.

Fixes: #577

Copy link

stackblitz bot commented Sep 16, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

vercel bot commented Sep 16, 2024

@MichalBryxi is attempting to deploy a commit to the pnpm Team on Vercel.

A member of the Team first needs to authorize it.

@MichalBryxi
Copy link
Author

MichalBryxi commented Sep 16, 2024

Happy to talk about any and all choices and alter things if necessary, but I firmly believe that this amount of verboseness greatly reduces user confusion as they don't have to guess what is possible / why / how.

```
pnpm add github:zkochan/is-negative
pnpm add bitbucket:pnpmjs/git-resolver
pnpm add gitlab:pnpm/git-resolver
Copy link
Author

Choose a reason for hiding this comment

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

Is there more? I found only those in tests.

* commit: `pnpm add kevva/is-positive#97edff6f525f192a3f83cea1944765f769ae2678`
* branch: `pnpm add kevva/is-positive#master`
* version range: `pnpm add kevva/is-positive#semver:^2.0.0`
```
Copy link
Author

Choose a reason for hiding this comment

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

Personally I prefer these triple-fenced examples, as they are way easier to target with a mouse for copy&pasting as you're constrained by the newlines. As opposed to the bullet point format above, which allows for copying the non-code prefix as well.

Copy link
Member

Choose a reason for hiding this comment

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

you can have both. code sections inside bullet points. I don't think there's a reason to move the descriptions into the code blocks.

Copy link
Author

Choose a reason for hiding this comment

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

No hard feelings, I can revert it, but to show why I'm against copy&paste examples in inline form: Here you can see how precise I have to be to select exactly what I intended:

2024-10-04 11 08 36

Compare it to the code-fence version that has the full line for mousing around and thus it's way harder to select unwanted things:

2024-10-04 11 10 59

Copy link
Author

Choose a reason for hiding this comment

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

I pushed the revert to adhere to the ways it was. Let me know if it works for you now.

Copy link
Member

Choose a reason for hiding this comment

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

But you could do it like this:

* Strict semver:
  
  `` `
  pnpm add zkochan/is-negative#semver:1.0.0
  `` `

Copy link
Author

Choose a reason for hiding this comment

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

Oooh, yeah why not. Pushed given change for both blocks. Also I used "Sentence case:" for the bullet points. Let me know if this works for you.


You may also install just a subdirectory from a Git-hosted monorepo. For instance:
You can specify version (range) to install using the `semver:` parameter. For example:
Copy link
Author

Choose a reason for hiding this comment

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

I consider the semver: and path: (below) being explicitly spelled out very important. It makes sure that the user knows it's a keyword and not just a random thing in given repo.

docs/cli/add.md Outdated
* branch: `pnpm add kevva/is-positive#master`
* version range: `pnpm add kevva/is-positive#semver:^2.0.0`
```
# latest commit from default branch
Copy link
Author

Choose a reason for hiding this comment

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

As far as I can tell pnpm does not look for master per-se, it looks for (in GitHub terminology) default branch, hence the naming change here.

pnpm add gitlab:pnpm/git-resolver
```

If `[provider]:` is omited, it defaults to `github:`.
Copy link
Author

Choose a reason for hiding this comment

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

IMO important to know the default here.

* commit: `pnpm add kevva/is-positive#97edff6f525f192a3f83cea1944765f769ae2678`
* branch: `pnpm add kevva/is-positive#master`
* version range: `pnpm add kevva/is-positive#semver:^2.0.0`
```
Copy link
Member

Choose a reason for hiding this comment

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

you can have both. code sections inside bullet points. I don't think there's a reason to move the descriptions into the code blocks.

docs/cli/add.md Outdated Show resolved Hide resolved
- Create sections to make visual/logical distinction of individual parts.
- Spell out all major possible syntaxes.
- Add more explanation to individual parts of the `URL` like `semver:` and `path:`.
- Copy & paste examples directly from [tests](https://github.com/pnpm/pnpm/blob/main/resolving/git-resolver/test/index.ts) so that we're reasonably sure they are correct.
- Explain defaults.
- Explain common cases like monorepo & forks.

Fixes: pnpm#577
- Adhere to current way of bullet pointed examples
@MichalBryxi MichalBryxi force-pushed the mb/577-improve-pnpm-add-git-url-docs branch from d3521c4 to 7c1d336 Compare October 4, 2024 09:21
- That and making the bullets sentence-case
- That and making the bullets sentence-case
Copy link

vercel bot commented Oct 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
pnpm-io ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 5, 2024 1:11am

@MichalBryxi
Copy link
Author

Preview looks reasonable (to me).

@zkochan
Copy link
Member

zkochan commented Oct 7, 2024

merged 2be27c0

@zkochan zkochan closed this Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve pnpm add <git remote URL> documentation
2 participants