-
Notifications
You must be signed in to change notification settings - Fork 1
feat(#17): rework cli interface #23
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
Conversation
92fc848 to
29df26b
Compare
scastlara
left a comment
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.
This is great! Thank you for this work 🚀
All comments are mostly nits, I am happy with this as is!
Maybe the README needs some love to show how to use this?
| zerolog.Warn().Msg("Testing mode enabled. This may enable features that are not safe for production use.") | ||
| publicChannelsEnabled = cCtx.Bool(publicSlackChannelFlag) | ||
| // Parse options | ||
| locations, err := parseUrls(cCtx.StringSlice(urlFlag)) |
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.
Ah, if only there was a way to cast/validate directly in the cli...
| // This function receives a list of paths which can be gitlab projects or groups | ||
| // and returns the list of projects within those paths and the list of projects contained within those groups and their subgroups. | ||
| func (s *service) gatherProjectsFromGroupsOrProjects(paths []string) (projects []gitlab.Project, warn error) { | ||
| for _, path := range paths { |
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.
shall we goroutine it?
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.
Agreed! But let's do this in a separate PR. I gave it a quick try and am not seeing our goroutine implementations super clear, maybe we can review them a little
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.
Totally!
29df26b to
47f3a18
Compare
This PR reworks the CLI interface as discussed in #17
What changed in the interface
testingflagpublic-slack-channel(rational: configurators of the cli should be responsible for using a public or private channelgitlab-groupsandgitlab-projectsunder a genericurlflag--urlflag accepts lists of URLS which have a supported platform scheme, and a project or group as the path. For examplegitlab://path/to/my/group, orgitlab://path/to/my/namespace, and soon to includegithub://user/projectreport-slack-channeltoreport-to-slack-channelreport-slack-project-channeltoreport-enable-project-report-toreport-gitlab-issuetoreport-to-issuereport-to-emailwhich does nothing for the moment.configandverboseWhat changed in gitlab logic
Now that paths can be either a group or a project, the logic was changed to:
The nice thing is that I found out that the gitlab API accepts the path instead of an actual ID, and this simplifies the gitlab logic we had in place 😄 (before we were using the search API to actually find the group, in order to use its real ID, etc.. so we were making more API calls than we now are)