Skip to content

Make it mandatory to also provide references when creating new validators? #2110

@pano9000

Description

@pano9000

Hello all,

Just wanted to bring this topic up, because I can't seem to find any rules about this yet:

I feel like it would be good practice to have the people who create a PR also provide some (credible) references for any new validators or additions to existing validators.

I.e. what information is the validation based on?

Those should ideally of course be some official docs, and not solely based on a (possibly outdated) Wikipedia entry.
I can see some PRs already providing this info, which is great and make it a lot easier to check, but then there are other ones, that do not.
In the latter case, you will just see a random RegExp that is supposed to check for something, and you won't know, is this valid or not.

There can be several reasons, where errors could result in a wrong RegExp, e.g.

  • based on outdated info
  • potentially misinterpreting or misreading the way the string is made up
  • accidentally making the RegExp "too permissive"

In those cased the (mandatory) provided tests of course can also be "faulty" as well.

Yes, providing references might make the PR process a tad bit more cumbersome, but I would rather say it is better to go for quality over quantity, especially for a validation library.

Implementing this will come with some benefits

  • it'll make it easier for people to actually review what the RegExp is supposed to test for
  • it'll increase confidence in the library doing correct validations

So I would propose to add this as an additional point to the "PR Checklist" at the very least.

What are your thoughts?

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions