search

beberlei / assert

Thin assertion library for use in libraries and business-model
Other
2.41k stars 186 forks source link

LazyAssertion not open for extension #284

Closed DannyvdSluijs closed 3 years ago

DannyvdSluijs commented 5 years ago

I wanted to extend the LazyAssertion in order to overload the that method. As I need to pass some additional details into the exception created by the assertion lib.

With all the properties on the LazyAssertion are private this doesn't allow for easy extending.

Would you be willing to accept a PR which would change the properties on the LazyAssertion from private too protected? (Like the Assert class already has, which can easily be extended)

rquadling commented 5 years ago

Happy to accept a PR. Please make sure you provide an example in a test covering the extension as an example usage. Also please read https://github.com/beberlei/assert/blob/master/CONTRIBUTING.md.

Looking forward to seeing this!

DannyvdSluijs commented 5 years ago

PR #285 has been created for this. If you could, at your convenience, have a look and let me know what you think of it.

I've made sure that:

DannyvdSluijs commented 5 years ago

@rquadling nudging you as a gentle reminder to get your feedback on my PR. Thanks in advance.

rquadling commented 5 years ago

Hi. Sorry! Looking.

rquadling commented 5 years ago

See the PR for my comments. Please rebase onto master for more uptodate pipeline checks and consistencies.

DannyvdSluijs commented 5 years ago

Thanks for the review, I'll pick it up from here.

rquadling commented 5 years ago

I've had to release a fix for some BC's. Can you rebase and fix please?

DannyvdSluijs commented 5 years ago

@rquadling will do a rebase but still havent found the time to makes changes based your review.

DannyvdSluijs commented 3 years ago

Leaving the PR for what it is. Ive no longer need the changes proposed in this PR. If somebody wants to take over feel free.