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

missing_presence_validation: ignore columns with defaults #171

Merged

Conversation

fatkodima
Copy link
Contributor

Fixes #170.

We can add a flag for this behaviour to the checker, but I do not think this will be needed, as this looks like a legit use case for all the cases.

@mhenrixon
Copy link

Cheers! Much appreciated image

@gregnavis
Copy link
Owner

I'd like to support @mhenrixon's use case, but at the same time I wouldn't like to force that on other users (including myself!). I suggest we add a new setting to missing_presence_validation such that:

  1. A non-global setting ignore_columns_with_default.
  2. The value of the new setting should be set to false to avoid breaking backwards compatibility.
  3. If the setting is true then we should get the behavior suggested by @mhenrixon; if it's false then we should get the old behavior.

Does that make sense, @mhenrixon, @fatkodima? Let me know if I mixed something up -- I'm terribly under-slept today.

@mhenrixon
Copy link

Does that make sense, @mhenrixon, @fatkodima? Let me know if I mixed something up -- I'm terribly under-slept today.

Sounds phenomenal @gregnavis! I can relate to the undersleeping 😩

@fatkodima fatkodima force-pushed the presence_validation-ignore-defaults branch from a5df8ae to 0d41954 Compare August 26, 2024 19:44
@fatkodima
Copy link
Contributor Author

Updated PR with ignore_columns_with_default new config option.

@gregnavis gregnavis merged commit 15a7d97 into gregnavis:master Aug 27, 2024
67 checks passed
@gregnavis
Copy link
Owner

Merged! Thank you @fatkodima and @mhenrixon. 🙇🏻

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.

Better handling of default values
3 participants