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

Proposed major change: delete And interface #6

Closed
wants to merge 7 commits into from

Conversation

dsaff
Copy link
Contributor

@dsaff dsaff commented Aug 24, 2011

This is moving the pull request #4 into a new branch, off of master, in order to stage multiple changes at once. Original text below, original comments at the URL above.

Coming back to Truth after a hiatus, my first impression is that we're paying a lot in type complexity, and not getting enough for it. In this case, I've been able to make the type system for all Subjects much simpler, including bringing CollectionSubject back from the brink of incomprehensibility, by dropping the and(). and() was fluency for fluency's sake, and I feel that the new tests are at most an iota less readable than the old ones, and only one test that once compiled no longer does. I don't necessarily expect an immediate LGTM, but wanted to generate an end-point for discussion.

One more point: this includes some bugfixes to Expect that I expect (ahem) to be uncontroversial, which we can consider on their own in pull request #7

@cgruber
Copy link
Contributor

cgruber commented Sep 22, 2011

Crap. Several things are getting mixed up, and probably partly because of my failure somewhere in merging. Grr.

@cgruber
Copy link
Contributor

cgruber commented Mar 15, 2012

Ok, so after a lot of soul searching, I think I can see my way clear to removing "And". I will miss it - I really liked that chaining approach, but I'm ok with it, despite the lesser englishy bit.

I think this has gotten mixed up between several changes, even though this was split out. Shall we try again?

@cgruber
Copy link
Contributor

cgruber commented Mar 15, 2012

Oh - this was submitted after all. Ok - got it. Apparently my own syncing failed on my clone.

@cgruber
Copy link
Contributor

cgruber commented Mar 15, 2012

Or no it wasn't. Dammit - too much time away from github - sorry for the spam. I'd liek to see this resolved and us move forward.

@cgruber
Copy link
Contributor

cgruber commented Mar 15, 2012

Ok, so I've pulled the other two pull requests that were mixed in to this - did you want to merge back into your branch, so we can see the final result without the extra bits about expect and fail?

@dsaff
Copy link
Contributor Author

dsaff commented Mar 29, 2012

Yeah, looks like I'll have to remerge. I'll try to do so tonight.

@cgruber
Copy link
Contributor

cgruber commented Mar 29, 2012

Heh. It's gonna hurt. :D

@dsaff
Copy link
Contributor Author

dsaff commented Apr 2, 2012

It looks like it will be difficult to remerge this without having to relive hours of typing mismatches that I've already lived through and forgotten once. I'm not really a consumer of Truth at the moment, so I'll let those who are active consumers decide whether my bias for shorter type parameter lists is truly worth the effort to get there. Shall I close this pull request?

@cgruber
Copy link
Contributor

cgruber commented Apr 2, 2012

Leave it open, but I'll take it as read that you're orphaning this effort - hagbard@, maybe you and I can hash this out to see if we want to pick it up or let it go.

On Apr 2, 2012, at 1:22 PM, David Saff wrote:

It looks like it will be difficult to remerge this without having to relive hours of typing mismatches that I've already lived through and forgotten once. I'm not really a consumer of Truth at the moment, so I'll let those who are active consumers decide whether my bias for shorter type parameter lists is truly worth the effort to get there. Shall I close this pull request?


Reply to this email directly or view it on GitHub:
#6 (comment)

@ghost ghost assigned cgruber Aug 1, 2012
@cgruber
Copy link
Contributor

cgruber commented Sep 4, 2012

New pull request created to deal with this, all synced with current at #37. I'll close this issue as obsolete (as an issue and pull request).

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