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

src: Fix usage/help text (and error message) for -b/-t flags for ppm install #105

Merged
merged 2 commits into from
Dec 14, 2023

Conversation

DeeDeeG
Copy link
Member

@DeeDeeG DeeDeeG commented Dec 13, 2023

These flags do not actually take commit SHAs as arguments, only branch or tag names.

The flags are aliases of one-another and work the exact same -- either flag can take a branch name or tag name. No SHAs.


Optional context: Issue report pulsar-edit/pulsar#821 over at the core repo brought this back to my attention, although I believe it had been brought up somewhere in Discord a long time ago, just now getting around to finally updating the usage/help text to be accurate.

These flags do not *actually* take commit SHAs as arguments,
only branch or tag names.

The flags are aliases of one-another and work the exact same --
*either* flag can take a branch name *or* tag name. No SHAs.
@DeeDeeG
Copy link
Member Author

DeeDeeG commented Dec 13, 2023

macOS CI failure is unrelated, it's the "old node-gyp, new Python" issues again.

I have to review how we worked around these things in CI for other repos. Wanted to get this PR out quickly, will have to come back around to fixing CI here soon.

EDIT: I am okay with delaying PRs such as this until CI is fixed, if folks prefer. We know how to work around these issues from experience at our other repos, so it just takes a quick look at that "prior art" and copy-pasting whatever workarounds seem the most palatable from there to here.

These flags don't accept commit hashes, only tag or branch names.
@DeeDeeG DeeDeeG changed the title src: Fix usage/help text for -b/-t flags for ppm install src: Fix usage/help text (and error message) for -b/-t flags for ppm install Dec 13, 2023
Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

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

As this is a text only PR, backed up by solid research, and behavior we are seeing error reports of, fully support this being merged in without any concern about the specs.

So thanks for touching this one, as like you mentioned, there is some confusion about it's promised feature set

@confused-Techie confused-Techie merged commit 2ecd97f into master Dec 14, 2023
8 of 11 checks passed
@Daeraxa
Copy link
Member

Daeraxa commented Dec 14, 2023

Just to add for posterity's sake, the docs look correct here: https://pulsar-edit.dev/docs/launch-manual/sections/using-pulsar/#command-line

It doesn't say a commit hash can be used:

pulsar -p install <git_remote> [-b <branch_or_tag>]

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.

3 participants