datree icon indicating copy to clipboard operation
datree copied to clipboard

Published 20 hours ago • verified publisher icon datreeio

Validate publish command args before cobra.Run

Open noaabarki opened this issue 3 years ago • 5 comments

Describe the solution you'd like

Validate test commands arguments before Run. This pattern will allow us to governance behaviors such as flags/arguments validation in all commands. Additionally, this should make the code easier to understand and maintain.

Requirements Golang basic level.

“How to Implement” suggestion

See issue as a reference.

noaabarki avatar Feb 24 '22 14:02 noaabarki

is anyone working on this issue?

just-injoey avatar Jul 06 '22 08:07 just-injoey

@just-injoey Still up for grabs, would you like to take it? :)

adifayer avatar Jul 06 '22 08:07 adifayer

Hi @adifayer I'm new to golang and would like give this a try. Can I take this?

anubha-v-ardhan avatar Sep 16 '22 18:09 anubha-v-ardhan

Hey @adifayer @noaabarki Can you tell me how we can achieve this, I want to work on this issue.

anubha-v-ardhan avatar Sep 18 '22 05:09 anubha-v-ardhan

@anubha-v-ardhan sure, you got it! Did you check out the issue that we added as a reference?

adifayer avatar Sep 18 '22 11:09 adifayer

Is this still in the works? after going through the referenced issue, I get the gist of what needs to be done, if anyone isn't working on this issue. I can take this up, can you give me your opinion on this @noaabarki?

KiranSatyaRaj avatar Nov 21 '22 12:11 KiranSatyaRaj

Hi @KiranSatyaRaj, this is still relevant, but we don't want to hijack @anubha-v-ardhan work 😇. @anubha-v-ardhan are you still working on it?

noaabarki avatar Nov 22 '22 12:11 noaabarki

Hey! There hasn't been any progress on this for a long time. Can I take this up? @noaabarki

rakshitgondwal avatar Mar 25 '23 16:03 rakshitgondwal

you're right. go for it!

eyarz avatar Mar 26 '23 09:03 eyarz

Hey @eyarz! Thanks a lot for assigning me this issue. I just went through the codebase and the other issue mentioned above. I had a question, where do I need to move the logic of validating the arguments in the cmd/publish.main.go?

rakshitgondwal avatar Mar 26 '23 12:03 rakshitgondwal

I think that the idea is to move the validation into the Args function. but it seems that it already happens there (only 1 argument, which is the path to the PaC.yaml file) https://github.com/datreeio/datree/blob/4cb4d835b8fe8f6b6abefedfad5220b860ec416a/cmd/publish/main.go#LL63C4-L69C5

@noaabarki is this issue still relevant?

royhadad avatar Mar 27 '23 15:03 royhadad

@rakshitgondwal, @royhadad, both of you are correct. Unfortunately, this issue has already been resolved and is no longer relevant. Sorry for the trouble.

noaabarki avatar Mar 28 '23 09:03 noaabarki