search

Tauffer-Consulting / domino

User friendly and open source platform for workflow creation and monitoring
https://domino-workflows.io/
Apache License 2.0
144 stars 14 forks source link

feat(cli): add create piece functionality #300

Closed tdari closed 4 months ago

tdari commented 5 months ago

This PR will add create piece functionality to domino-cli. Users will be able to create boilerplate code necessary for the new pieces using this command: domino piece-repository pieces create --name <name>

nathan-vm commented 5 months ago

This looks like a great feature, well done @tdari 🚀

tdari commented 5 months ago

Hey @tdari this is really good and useful, thanks for submitting. What you think about removing the pieces cli from the piece-repository nesting? I think we don't need this dependency since the CLI will execute on the current folder. This usage looks simpler to me:

domino pieces --create <name>

What you think about that?

I also think the current version is a little bit made it longer, your suggestion can work but I think the cli shouldn't be dependent on the directory it has been called (even though its dependent right now). We can add a repository option to specify which repository this new piece will be added to, like, domino repository pieces create --repository <path_to_repo> --name <name> this will give us flexibility and programmability of the cli in the future use cases IMO. For that reason, I think we should specify the pieces command under the repository because there is a hierarchy. Maybe we can work on a different pr and re-design cli specifications. I was planning to refactor cli in that regard. I'd like to hear your opinions too.

vinicvaz commented 5 months ago

@tdari I agree with that accepting repository path to the pieces creation is the best. I'm not sure yet about the need of the dependency on piece repository CLI in this case, but by now lets keep the hierarchy you mentioned. Something like should be enough:

domino piece-repository pieces create --name <name> --repository_path <path> 
# optional path, if none use cwd

Are you planning to add the path argument in this PR?

Also, feel free to send a PR including this in the docs here https://github.com/Tauffer-Consulting/domino-docs

tdari commented 5 months ago

@vinicvaz yes I'll re-open this again including all the revisions. Thank you.

vinicvaz commented 4 months ago

@tdari great you are adding tests! Also, I discussed with @luiztauffer and we decided to go on with the simpler (less verbose) CLI without the nested. So I think this PR can update the following: domino pieces to domino pieces-repository to store all CLI actions for pieces repositories and domino pieces-repository pieces to domino pieces to store all actions related to pieces. We can pass the repository as an argument like described before. an example of use would be domino pieces create --name my-piece --repository_path path/to/repo With this we keep it more concise and easier

tdari commented 4 months ago

@vinicvaz it looks good to me. what about making it even shorter by using just repository instead of piece-repository. domino repository pieces, I think users will have enough context to recognize that is about piece repository. We can clearly indicate that in help messages. What do you think?

vinicvaz commented 4 months ago

I aggree users probably will understand but as we are using Pieces Repository in almost everywhere (docs, code, etc) I think it will be better to keep pieces-repository . Basically applying the explicit better than implicit rule for this

luiztauffer commented 4 months ago

nice work here @tdari !

tdari commented 4 months ago

Thanks, @vinicvaz. I checked the docs as you suggested, but I didn't want to add the new feature to the docs since there was only a paragraph mentioning the CLI. We can add it later once the CLI becomes mature enough, or we can write a post in the blog section to show users how they can create their pieces and mention these features there.