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

Adding ErrorAssertionFunc function type #155

Merged
merged 3 commits into from
Jun 5, 2024
Merged

Conversation

ccarstens
Copy link
Contributor

Hi @shoenig, thank you for creating this formidable assertion lib! :)

The type I added is analog to testify's ErrorAssertionFunc type that makes it convenient to pass Error or NoError assertion functions in table driven tests.
Do you like the idea of adding this type?
I wasn't sure what the best placement of the type in the source file would be, so please let me know if you wanna move it somewhere else.
Best,
Cornelius

@shoenig
Copy link
Owner

shoenig commented Jun 5, 2024

Hi @ccarstens! I can see why this would be useful and would be glad to add it. I would just want to see two things - a doc string explaining what the type is useful for, and once that's in place, run make generate to make sure it gets copied to the generated must package. Thanks!

@ccarstens
Copy link
Contributor Author

ccarstens commented Jun 5, 2024

Hi @shoenig! Ok that sounds great! I added a godoc comment and I ran make generate.
I'm on macOS and the sed commands don't work out of the box like this.
Are you interested in the addition of a separate make generate-macos function? If so, I'm happy to add it with sed commands that work on macOS.

@shoenig
Copy link
Owner

shoenig commented Jun 5, 2024

Thanks @ccarstens! I created #156 with some thoughts about fixing the BSD sed problem. The script could definitely be refactored, since it's just applying the same 3 operations to a list of those 11 files. But just making it so sed does the right thing depending on the version is also an option. In any case we have that separate issue now ~

@shoenig shoenig merged commit 9ebcf5a into shoenig:main Jun 5, 2024
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