-
Notifications
You must be signed in to change notification settings - Fork 319
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
Add npm list --json
to /admin
#318
Conversation
This really has no business in that route. Make a new route specifically for it if you must. |
It is an admin route for sure but if you want a separate path to handle this that is fine. The objection means we won't be reusing code.
Fully referenced with the Applies to... |
You're not reusing any of the route's unique logic. https://github.com/OpenUserJs/OpenUserJS.org/blob/master/controllers/admin.js#L135-L146 You're just hacking a conditional into it's logic to completely change it's function. The route is for getting raw data from the database. That and nothing else. Copy+pasting permission checks on the authedUser is fine. Sooner or later it'll move into middleware but for now, being consistent with all the other routes is good. |
The API Keys are not exclusively related to our DB. The npm DB is just another DB just like GH, Googles, etc. Anyhow I've moved it to another sub-path of admin (route) but it's currently staying in the same controller since it is an admin function and not specific to the admin panel for a script. I appreciate your explanation albeit perceived a bit gruff. P.S. There's also a chance that |
Add a column under dynamic to denote type for our current release
Whoops reopening. |
arggh going to need to resubmit this. Somehow messed up the mergability. |
* Show the raw npm ls JSON data for the server Applies to OpenUserJS#296 This appears to be the only way it can be done for listing the current dependencies. I haven't found any native API to Node.js that does this... so I'm willing to give this a whirl.
* Added `try...catch` just in case it's a fatal error **NOTES** ``` sh-session problems": [ "peer dep missing: kerberos@~0.0, required by [email protected]" ], ``` ... this shows up in the stdout as being the "error" along with the full compliment of packages installed ... see OpenUserJS#832 and parent OpenUserJS#318 *(never would have guessed this on nodejitsu... glad we aren't there anymore)*
Applies to #296
This appears to be the only way it can be done for listing the current dependencies. I haven't found any native API to Node.js that does this... so I'm willing to give this a whirl.
@Zren and @OpenUserJs/owners any objections?