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

Add npm list --json to /admin #318

Closed
wants to merge 1 commit into from

Conversation

Martii
Copy link
Member

@Martii Martii commented Aug 23, 2014

  • Tack on a virtual model name since we have no control over this implied native DB model.

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?

@Zren
Copy link
Contributor

Zren commented Aug 23, 2014

This really has no business in that route. Make a new route specifically for it if you must.

@Martii
Copy link
Member Author

Martii commented Aug 23, 2014

This really has no business in that route.

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.

What is this for anyways? Showing the current package versions?

Fully referenced with the Applies to...

@Zren
Copy link
Contributor

Zren commented Aug 23, 2014

The objection means we won't be reusing code.

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.

@Martii
Copy link
Member Author

Martii commented Aug 23, 2014

The route is for getting raw data from the database.

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 exec is locked on the drones too so we'll see what happens. This is much better than just guessing at what the differences are between dev and production.

Martii pushed a commit to Martii/OpenUserJS.org that referenced this pull request Aug 23, 2014
@Martii Martii closed this Aug 23, 2014
@Martii Martii deleted the documentPageFixes branch August 23, 2014 11:40
@Martii Martii restored the documentPageFixes branch August 23, 2014 11:41
@Martii Martii deleted the documentPageFixes branch August 23, 2014 11:41
Add a column under dynamic to denote type for our current release
@Martii
Copy link
Member Author

Martii commented Aug 23, 2014

Whoops reopening.

@Martii Martii reopened this Aug 23, 2014
@Martii Martii removed the PR READY label Aug 23, 2014
@Martii
Copy link
Member Author

Martii commented Aug 23, 2014

arggh going to need to resubmit this. Somehow messed up the mergability.

@Martii Martii closed this Aug 23, 2014
Martii pushed a commit to Martii/OpenUserJS.org that referenced this pull request Aug 23, 2014
* 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.
Martii added a commit that referenced this pull request Aug 23, 2014
@Zren Zren mentioned this pull request Dec 2, 2014
Martii pushed a commit to Martii/OpenUserJS.org that referenced this pull request Nov 19, 2015
* Added `try...catch` just in case it's a fatal error

**NOTES**

``` sh-session
problems": [

    "peer dep missing: kerberos@~0.0, required by mongodb-core@1.2.21"

],
```

... 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)*
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. DOC Pertains inclusively to the Documentation operations. enhancement Something we do have implemented already but needs improvement upon to the best of knowledge.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants