Skip to content

Conversation

@DannyBen
Copy link
Member

@DannyBen DannyBen commented Oct 23, 2021

Checklist

  • Improve validation error message
  • Add file_exists validation
    • and test in fixtures
  • Add dir_exists validation
    • and test in fixtures
  • Add not_empty validation
  • Remove support for array validations

Closes #132

@DannyBen
Copy link
Member Author

In addition - I am also wondering if at all we need to allow validate to receive an array. I cannot find a single use case where this is useful.

@DannyBen DannyBen added this to the v0.6.9 milestone Oct 23, 2021
@wolfgang42
Copy link
Collaborator

wolfgang42 commented Oct 23, 2021

In addition - I am also wondering if at all we need to allow validate to receive an array. I cannot find a single use case where this is useful.

I left an example in the original issue, of [is_readable, is writable], but I agree that this is a vague use case for me too. Maybe to simplify, at least for now, these can simply be defined as custom validations:

validate_scratch_file() {
    validate_is_file "$1" && validate_readable "$1" && validate_writable "$1"
}

@DannyBen
Copy link
Member Author

I left an example in the original issue, of [is_readable, is writable]

Well, supporting an array is not a big deal and is already implemented, it just adds some confusion to the docs, and I am not sure it is needed. We can keep it for now but it is on the chopping block.

@wolfgang42
Copy link
Collaborator

If it's on the chopping block I'd say take it out; it can always be added back in later if someone has a need or comes up with a clearer way to do it.

Copy link
Member Author

@DannyBen DannyBen left a comment

Choose a reason for hiding this comment

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

Ready to be merged from my standpoint.

@DannyBen DannyBen mentioned this pull request Oct 23, 2021
4 tasks
@DannyBen DannyBen merged commit f0742b7 into master Oct 25, 2021
@DannyBen DannyBen deleted the add/validations branch October 25, 2021 05:14
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.

Add some built in validations

3 participants