Closed cgruber closed 13 years ago
Thanks for all of this work. There's a ton going on here, so I've commented in many places. It's generally the right direction, and I'm open to turning open or hairy issues into TODOs, and handling those in future commits/pulls.
I'm more than happy to do it. It's been awesome. I sort of expected you to pull the moreasserts branch first, then do this chaining stuff, but are you ok just pulling in this branch? It incorporates the moreasserts branch's changes, except for a few which I can roll forward.
So, it looks to me like we've got the following open issues:
Open issues:
int/long/boolean vs Integer/Long/Boolean
null.contains(null)
is(T)
Make success a strategy (for "should fail" tests)
Example test file
Unneeded uses of new String[]{} in tests
ObjectSubject (I thought I commented on this, but the comment seems to have gotten lost; we can re-address later).
At this point, we do need to close out this branch (are you intentionally adding new stuff like BooleanAsserts in mid-discussion?) So if you agree that I haven't missed anything that's still open, I'll merge, and then we can start knocking off those issues one by one.
Sorry about the boolean asserts - needed them in another set of changes, and they were from "moreasserts" originally. I think i've got most of the assertions we need in for self-testing (using truth to test truth). All other asserts would be for external customers, so I'll stop adding them to this branch.
I agree with the list of issues below except that unneeded uses of new String[]{} should be handled now. If not, then I just missed some spots. Post-merge I'll do a global search for them. Not sure I know what ObjectSubject is, unless you mean DefaultSubject, which was just used to match on Object - or if you mean the whole question of matching on Object at all.
There are a few more things to address - the delegation approach (which I have working in another branch), but I think the issues in this branch are reflected here, or handled already.
Christian.
On Jun 8, 2011, at 10:51 AM, dsaff wrote:
So, it looks to me like we've got the following open issues:
Open issues:
int/long/boolean vs Integer/Long/Boolean
null.contains(null) is(T) Make success a strategy (for "should fail" tests) Example test file Unneeded uses of new String[]{} in tests ObjectSubject (I thought I commented on this, but the comment seems to have gotten lost; we can re-address later).At this point, we do need to close out this branch (are you intentionally adding new stuff like BooleanAsserts in mid-discussion?) So if you agree that I haven't missed anything that's still open, I'll merge, and then we can start knocking off those issues one by one.
Reply to this email directly or view it on GitHub: https://github.com/dsaff/truth/pull/10#issuecomment-1327198
Note - this pull request is based on moreasserts branch, so most of these commits will already be in if we merge the other pull request.