Adds ability to list installed plugins from CLI #5920
Conversation
|
Looks like there's something funky going on here, the PR includes 3 commits that are already in master. |
| @@ -0,0 +1,18 @@ | |||
| const fs = require('fs'); | |||
|
|
|||
| module.exports = { | |||
There was a problem hiding this comment.
Use an es6 export instead of commonjs, so:
export function list(settings, logger) {instead of module.exports = {...}.
There was a problem hiding this comment.
Okay, will make this change in this file. Should I also make the same change in the other plugin_*.js files in this directory (that aren't in the scope of this PR)?
There was a problem hiding this comment.
No need to modify the other files. We plan to run through the entire codebase before Kibana 5 to update the majority of the existing commonjs module stuff.
|
Bunch of bits of feedback there, most of which are stylistic. The only bug I found was the conditional check for multiple commands at once. |
| ++numChosen; | ||
| } | ||
| }); | ||
| return (numChosen > 1); |
There was a problem hiding this comment.
I'd recommend doing this by checking the length of the intersection rather than imperatively counting the duplicates. You can import intersection from the lodash module, and then this function becomes:
function areMultipleOptionsChosen(options, choices) {
return intersection(Object.keys(options), choices).length > 1;
}There was a problem hiding this comment.
Beautiful. Changing. Thanks!
|
I've tested this out and it works well. My latest round of feedback is all stylistic recommendations rather than blockers, so this LGTM. |
|
@ycombinator Just assign this issue to me when you've addressed whatever feedback you want to address. |
| @@ -1,5 +1,6 @@ | |||
| const { resolve } = require('path'); | |||
| const expiry = require('expiry-js'); | |||
| import intersection from 'lodash'; | |||
There was a problem hiding this comment.
This should actually be import { intersection } from 'lodash'; The current line should error.
There was a problem hiding this comment.
Correction, the current line itself wouldn't error, however intersection would be the lodash constructor itself rather than the intersection function from within lodash.
Adds ability to list installed plugins from CLI
|
Awesome, thanks for doing this! |
Closes #5916
Note: The code in this PR only goes as far as listing the names of the installed plugins. It does not list their version numbers. Please let me know if that's something worth adding.