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

Added GUI, Search and Add User #73

Closed
wants to merge 46 commits into from
Closed

Conversation

bpeacock
Copy link

I added a page at the root that displays the local packages and allows users to search through them. The page shows basic information about each package including its name, author, description and README. I also added the capability to add users from the command line. This involved removing user configuration from the config.yaml and using a users.json file that is written only by the Sinopia server.

This is a large set of changes that will require some additional work before merging. However, I wanted to get the process started to make sure that the new code is on track. Things that still need to be changed:

  • Add tests for new functionality
  • Add instructions for development. (How are you compiling the package.yaml to json for installs?)
  • Add user permissions to the new user model

Thanks for your help and hopefully we can get this code integrated into the project!

@rlidwka
Copy link
Owner

rlidwka commented Jun 22, 2014

Thank you! It's definitely something I'd like to see in the project.

I'm a bit worried about npm logo, it might be copyrighted.

This involved removing user configuration from the config.yaml and using a users.json file that is written only by the Sinopia server.

I'm planning to move it to htpasswd format instead to make it interoperable with nginx (will push the code in a few days). I assume it would work just as well for you?

Add instructions for development. (How are you compiling the package.yaml to json for installs?)

I'm using yapm, which understands yaml as is. For example, this works:

$ sudo npm -g install yapm
[sudo] password for alex: 
/usr/bin/yapm -> /usr/lib/node_modules/yapm/bin/npm-cli.js
yapm@1.4.0 /usr/lib/node_modules/yapm

$ yapm install git+https://github.com/rlidwka/sinopia.git
  http - git https://github.com/rlidwka/sinopia.git#master
  http - GET https://registry.npmjs.org/express
  http - GET https://registry.npmjs.org/commander
^C

I know some people who use npm-yaml instead.

(sorry for the late answer, was on vacation)

@bpeacock
Copy link
Author

Hi there, glad to hear from you! @jdr0dn3y, @ilowe and myself have been working off of a fork of your project because we weren't sure if this was still being maintained. As for your comments:

NPM Logo - It appears that the logo is not protected by a trademark and while it would be subject to copyright, we are using a significantly modified version of it and the original is checked into the NPM website repo which is licensed as BSD. Since we are using the logo to denote that this is a system compatible with NPM, I don't think that that creates confusion among consumers of the respective libraries. With those points in mind, I feel pretty comfortable using the logo. That being said, I'm not terribly attached to the modified logo that I created.

htpasswd - Sounds like a good plan--I just needed a format that could be written programmatically easily and wanted to separate it out from the configuration.

yapm - I will add that to the readme so that its easier for people to get setup.

@iainlowe
Copy link

Hi guys,

Some comments:

htpasswd - I think it's nice to have everything self-contained. As long as pnpm/sinopia is able to consume the format natively (ie. without help from nginx) I'm +1 on this. I'd love to see pluggable auth one day, but I'm not sure this should really be a focal point.

yapm - This is cool, but we should definitely not drop support for people using "standard" tools (ie. npm in this case). So I think it's OK to have the package.yaml but we should generate and commit the package.json so that dev'ing on our project does not require extra stuff. The problem (as I see it) is that even boot-strapping yapm in requires a standard package.json. I don't think we should force people to use a million extra tools just to get a build out of the box. FYI, the install line above works with npm properly (it's how I build my docker image).

repo - let's make the move fully to @jdr0dn3y repo as discussed over email.

I'll probably send a PR that includes my "blessed packages" feature (discussed over email I think) in the next week or so.

@coreybutler
Copy link

Any progress on merging this? I almost started duplicating this functionality until I happened across this PR. Would love to see this in a release.

@iainlowe
Copy link

iainlowe commented Jul 2, 2014

Hi @coreybutler we have had no luck in contacting the original author of this project so @bpeacock @jdr0dn3y and myself have been working off Justin's fork. You can find it here: https://github.com/jdr0dn3y/sinopia

At the moment, there is no indication that this stuff will actually get folded into the original project. I recommend you start following the fork I listed since that's where we are actually fixing issues and moving ahead.

@dsuckau
Copy link

dsuckau commented Jul 24, 2014

Would be interesting to know if this PR will be merged?

@rlidwka
Copy link
Owner

rlidwka commented Jul 26, 2014

Merged in 4f913f2 . Thanks!

It still needs some kind of an access control though.

@rlidwka rlidwka closed this Jul 26, 2014
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.

5 participants