search

workshopper / how-to-npm

A module to teach you how to module.
https://www.npmjs.com/package/how-to-npm
ISC License
1.09k stars 212 forks source link

feat: uses verdaccio instead mock manually the registry #143

Closed juanpicado closed 4 years ago

juanpicado commented 5 years ago

I would like to file a proposal.

I've noticed the mock registry was quite complicated to maintain and quite old, thus since I am Verdaccio maintainer https://verdaccio.org/ I decided to proceed to replace such implementation with the verdaccio registry.

https://www.npmjs.com/package/@verdaccio/how-to-npm

I have a PoC ready to use for code review.

npm install -g @verdaccio/how-to-npm

For the user nothing changes, all the problems are not affected at all, Verdaccio runs under the hood, in the same way, the previous implementation, it will reduce the amount of code to maintain, full npm registry experience and a UI where users can browse for more visual feedback about what is on the registry while they proceed with the tasks.

~In this PoC I'm using "verdaccio": "4.0.0-alpha.7" since us bug-free and more updated with the current npmjs endpoints, but the latest version can be used also perfectly.~ The latest is already v4.

Furthermore, new problems might be added since Verdaccio is adding constantly new endpoints as search, starts and soon tokens.

juanpicado commented 4 years ago

Would this be interesting for anyone ? Otherwise I would close this PR :(

EmmaRamirez commented 4 years ago

@juanpicado Don't close it! This is awesome! I'll take a look at it, but it looks good

@watilde perhaps you might wanna review this as well?

juanpicado commented 4 years ago

Nice 🎉 . I'll update the dependency, it is a bit old already.

watilde commented 4 years ago

Hello! Thank you for your big effort. To help me understand more about the purpose of this PR as refactoring for better maintenance, I have a small comment since I'm wondering if this PR is adding more lines instead of reducing.

+4,645 −257

I see an improvement at lib/registry.js, but do we need to add more mock files to maintain as a tradeoff?

juanpicado commented 4 years ago

If I use single line for metadata and then remove lock file, you will get more red than green.

Let me know whether I should use single line, but IMHO, I would leave them prettified because are more readable, it is handy to see what's is inside, specially in code reviews.

ccarruitero commented 4 years ago

I think would be better for review to move prettifier changes to other PR.

Aside other comments the code seems good.

juanpicado commented 4 years ago

done 👍

ccarruitero commented 4 years ago

@juanpicado thanks for all the work you put on this PR.

Aside my comments, just ensure assets files aren't modified if not necessary and please squash your commits.

juanpicado commented 4 years ago

@ccarruitero done 👍

ccarruitero commented 4 years ago

@juanpicado sorry for delay. Thanks for all the work you did.

I think the assets files should be just renamed, but there are some that are modified. Also, check this comment for the style in the require line

juanpicado commented 4 years ago

No problem, I will take care of that 👍

juanpicado commented 4 years ago

Sorry the delay, I revisited all change requested, ready for next one :)