Skip to content
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

fix(core): disable check args exist valid for raw #788

Merged
merged 6 commits into from
Mar 24, 2020
Merged

fix(core): disable check args exist valid for raw #788

merged 6 commits into from
Mar 24, 2020

Conversation

Sh4d1
Copy link
Contributor

@Sh4d1 Sh4d1 commented Mar 18, 2020

Fixes #787

Signed-off-by: Patrik Cyvoct [email protected]

Copy link
Contributor

@loicbourgois loicbourgois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test, so we're sure this bug doesn't come back ?

Signed-off-by: Patrik Cyvoct <[email protected]>
@@ -71,6 +88,25 @@ func Test_handleUnmarshalErrors(t *testing.T) {
}))
}

func Test_RawArgs(t *testing.T) {
Copy link
Contributor

@loicbourgois loicbourgois Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shoudn't we have a testcobraRun func that only test cobraRun ?

I find it strnage that we hav to test an entire command.

Shouldn't we only call cobraRun() in the test ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could 😄 I'll fix that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though it may be more complicated 🤔

Signed-off-by: Patrik Cyvoct <[email protected]>
Copy link
Contributor

@kindermoumoute kindermoumoute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@loicbourgois loicbourgois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@loicbourgois loicbourgois merged commit 620c6f3 into v2 Mar 24, 2020
@kindermoumoute kindermoumoute deleted the fix_787 branch March 24, 2020 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants