Skip to content

Conversation

fredbi
Copy link
Member

@fredbi fredbi commented Sep 28, 2025

github.com/asaskevich/govalidator hasn't evolved in the recent years.

It has proved a useful dependency so far, but now we prefer to duplicate the few lines of codes that we were still using so we are more in control of the validations being carried out.

In particular, we may reduce further the footprint of regexp-based validations.

Change type

Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update

Short description

Fixes

Full description

Checklist

  • I have signed all my commits with my name and email (see DCO. This does not require a PGP-signed commit
  • I have rebased and squashed my work, so only one commit remains
  • I have added tests to cover my changes.
  • I have properly enriched go doc comments in code.
  • I have properly documented any breaking change.

github.com/asaskevich/govalidator hasn't evolved in the recent years.

It has proved a useful dependency so far, but now we prefer to duplicate
the few lines of codes that we were still using so we are more in
control of the validations being carried out.

In particular, we may reduce further the footprint of regexp-based
validations.

Signed-off-by: Frederic BIDON <[email protected]>
Copy link

codecov bot commented Sep 28, 2025

Codecov Report

❌ Patch coverage is 92.56198% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.12%. Comparing base (900913b) to head (2517cba).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
default.go 92.56% 6 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #176      +/-   ##
==========================================
+ Coverage   82.86%   83.12%   +0.26%     
==========================================
  Files          13       13              
  Lines        2188     2276      +88     
==========================================
+ Hits         1813     1892      +79     
- Misses        296      302       +6     
- Partials       79       82       +3     
Flag Coverage Δ
oldstable-macos-latest 83.12% <92.56%> (+0.26%) ⬆️
oldstable-ubuntu-latest 83.12% <92.56%> (+0.26%) ⬆️
oldstable-windows-latest 83.12% <92.56%> (+0.26%) ⬆️
stable-macos-latest 83.12% <92.56%> (+0.26%) ⬆️
stable-ubuntu-latest 83.12% <92.56%> (+0.26%) ⬆️
stable-windows-latest 83.12% <92.56%> (+0.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fredbi fredbi merged commit 5e4146b into go-openapi:master Sep 28, 2025
11 checks passed
@fredbi fredbi deleted the chore/remove-govalidator branch September 28, 2025 12:17
Copy link
Contributor

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

While I get the point here, there is a problem.

You were relying on go-validate, so on their tests.

Now, you import code but not the tests, so if you update a regexp for whatever reason, it would fail.

Note, these are unexported methods. So maybe your code is already covered by a full test suite. I didn't check.

But I prefer being replied "nah, it's fine" than seeing in tested code being imported.

@fredbi
Copy link
Member Author

fredbi commented Sep 28, 2025

I am sorry @ccoVeille I didn't get your comment in time.
Yes we have our own tests. They may not covr all edge cases like the original, but I think this is good enough for us.
Most stuff here that I'have replaced should never have been added in the first place (stuff like RGBColor, etc), but now we have it so I am maintaining it. I'll make another pass to cross-check coverage with asaskevitch/govalidator but I think we should already cover most cases.

Thanks

@fredbi
Copy link
Member Author

fredbi commented Sep 28, 2025

@ccoVeille now working on the mongo driver dependency issue, adopting a strategy like the one I suggested yesterday for testify: copy code, freeze and deprecate, move new feature to separate module. Will probably need to split the work in a couple pull requests

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.

2 participants