Skip to content

Apply better support info for Verdaccio 4#38

Merged
n4bb12 merged 8 commits inton4bb12:masterfrom
pmmmwh:better-support-info
Nov 22, 2019
Merged

Apply better support info for Verdaccio 4#38
n4bb12 merged 8 commits inton4bb12:masterfrom
pmmmwh:better-support-info

Conversation

@pmmmwh
Copy link
Copy Markdown
Contributor

@pmmmwh pmmmwh commented Nov 19, 2019

Hi! I really appreciate the work done in this plugin and we are currently using this for our own private registry. However - we realized a few things that are misleading to the users. This PR aims to fix that presentation issue so that the registration of the GitHub OAuth flow is easier.

Patches applied in the PR:

  • Added express as a dependency instead of a peer dependency because the CLI needs it (npx will not install devDependencies nor peerDependencies)
  • Added better registry URL parsing from the global VERDACCIO_API_URL so that clicking for info in the package page won't get the wrong URL
  • Added proper parsing for the info shown, with each command as a new line, as well as support for other package bundlers (npm as is, also for pnpm and yarn)

Copy link
Copy Markdown
Owner

@n4bb12 n4bb12 left a comment

Choose a reason for hiding this comment

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

Thanks for the improvements 🏕

n4bb12
n4bb12 previously approved these changes Nov 20, 2019
@n4bb12
Copy link
Copy Markdown
Owner

n4bb12 commented Nov 20, 2019

One more thing. I think a gif explains best. The registry URL seems to differ based on what URL the page is loaded on.

hmm

@pmmmwh
Copy link
Copy Markdown
Contributor Author

pmmmwh commented Nov 21, 2019

One more thing. I think a gif explains best. The registry URL seems to differ based on what URL the page is loaded on.

hmm

I think that is a bug on Verdaccio's side. This function here uses location.pathname, and is triggered on header load, so it will fail if the header is first loaded in any pages other than root.

I filed a PR upstream here

Edit: Oops, I discovered a bug - I will fix that.

Copy link
Copy Markdown
Owner

@n4bb12 n4bb12 left a comment

Choose a reason for hiding this comment

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

Thank you ✨

@n4bb12 n4bb12 merged commit 3e09b7f into n4bb12:master Nov 22, 2019
@n4bb12
Copy link
Copy Markdown
Owner

n4bb12 commented Nov 22, 2019

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.

2 participants