-
Notifications
You must be signed in to change notification settings - Fork 40
Feat: add recursive group search #37
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
base: master
Are you sure you want to change the base?
Conversation
This explains why I couldn't find some things, I'd love to see this added! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I didn't understand was how the version of gitlab-search
installed via npm worked for finding a project belonging to a subgroup since I only specified the group
@@ -10,7 +10,6 @@ when needed. | |||
2. Create a [personal GitLab access token](https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html#creating-a-personal-access-token) with the `read_api` scope. | |||
|
|||
## Installation | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: don't remove this newline.
@@ -63,6 +63,7 @@ Commander.( | |||
"-g, --groups <group-names>", | |||
"group(s) to find repositories in (separated with comma)", | |||
) | |||
|> option("-r, --recursive", "Search recursively in projects in the given groups") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: lowercase is used for the other options (also update README)
## Installing from source | ||
1. Install node 14.x | ||
2. Clone this repository | ||
3. Build with | ||
```sh | ||
$ npm run build | ||
``` | ||
4. Run from bin directory | ||
```sh | ||
$ bin/gitlab-search.js -h | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be a separate commit?
The prerequisites don't list a specific NodeJS version, and it doesn't seem to be required from what I tested.
Some news? @phillipj |
Hi @marsluca, Any improvements going forward, will have to be made on the TypeScript rewrite: #45. If you could help out giving that beta version a try, I'd be grateful as it would set the stage for merging & publishing a new version, before consider porting these changes into TypeScript. In case you're not interested in trying it out, I'd suggest forking the project and adding these changes into your version of it. |
fix #34