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.

Sign up for GitHub

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

Jump to bottom

Consider renaming Dont* types #82

Open
glen-84 opened this issue Jun 16, 2022 · 3 comments · May be fixed by #84
Open

Consider renaming Dont* types #82

glen-84 opened this issue Jun 16, 2022 · 3 comments · May be fixed by #84
Labels
enhancement New feature or request
Milestone
11.0.0

Comments

@glen-84
Copy link

glen-84 commented Jun 16, 2022

I don't (😉) love the use of contractions in type/variable names. What do you think about alternatives like:

DontValidateAttribute -> DisableValidationAttribute -> f.e. [DisableValidation]
DontValidateImplicitlyAttribute -> DisableImplicitValidationAttribute -> f.e. [DisableImplicitValidation]

?
@benmccallum
Copy link
Owner

Yea it did feel a bit dirty when I did it, but I needed to get it out :P

I'd be happy to take a PR if you're up for it? [Obsolete]ing the current one and creating a new one, copy pasting is fine,

What about [NoValidation] to go even shorter?

@benmccallum benmccallum added the enhancement New feature or request label Jun 17, 2022
@benmccallum benmccallum added this to the 9.1.0 milestone Jun 17, 2022
@glen-84
Copy link
Author

glen-84 commented Jun 17, 2022 •
edited
Loading

Hmm.

The only consideration would be the naming of the corresponding fluent API, and whether or not you want those methods to all be verbs.

.Argument("foo").ValidateWith<FooValidator>(); // verb
.Argument("foo").NoValidation(); // not a verb
.Argument("foo").NoImplicitValidation(); // not a verb

vs

.Argument("foo").ValidateWith<FooValidator>(); // verb
.Argument("foo").DisableValidation(); // verb
.Argument("foo").DisableImplicitValidation(); // verb

I think I prefer the latter, but it's up to you.

@benmccallum
Copy link
Owner

Good point. I think Disable would be the way to go. Since its opt-in by default that makes sense to "disable" the default.

@glen-84 glen-84 linked a pull request Jun 25, 2022 that will close this issue
Open
@benmccallum benmccallum modified the milestones: 10.0.0, 11.0.0 Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
11.0.0
Development

Successfully merging a pull request may close this issue.

Rename Dont* types glen-84/fairybread
2 participants
@benmccallum @glen-84