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

added excluded_if/excluded_unless tags + tests #847

Merged
merged 4 commits into from May 1, 2022

Conversation

jh125486
Copy link
Contributor

Fixes Or Enhances

Adds excluded_if and excluded_unless tags with tests.

Make sure that you've checked the boxes below before you submit PR:

  • Tests exist or have been written that cover this particular change.

@go-playground/validator-maintainers

@coveralls
Copy link

coveralls commented Oct 14, 2021

Coverage Status

Coverage increased (+0.04%) to 75.171% when pulling b64924c on jh125486:issue780_excluded_extras into 58d5778 on go-playground:master.

@agbishop
Copy link

This would be useful, can someone please review this?

@jh125486 jh125486 requested a review from a team as a code owner April 19, 2022 13:21
Copy link
Contributor

@deankarn deankarn left a comment

Choose a reason for hiding this comment

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

Looking great overall, does it work with nested fields though?

eg. If one wanted to require unless field in a parent or child structs field?

baked_in.go Outdated
if len(params)%2 != 0 {
panic(fmt.Sprintf("Bad param number for excluded_unless %s", fl.FieldName()))
}
fmt.Println(params[0], params[1], requireCheckFieldValue(fl, params[0], params[1], false), !hasValue(fl))
Copy link
Contributor

Choose a reason for hiding this comment

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

Leftover debug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, 100% a leftover. Pushed with changes.
I'll add a test for nested fields today (hopefully).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, tests for nested (inner) structs were added.

Golangci-lint is very angry though, but locally runs fine (I'm running 1.45.2 though).
I also don't see a golangci lint yaml config in the root dir...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@deankarn anything else I need to do for this?

@deankarn deankarn merged commit c0195b2 into go-playground:master May 1, 2022
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.

None yet

4 participants