Skip to content

Conversation

@stephencelis
Copy link
Member

This PR introduces a newer, flatter way of asserting with the test store. Instead of calling store.assert(...) with a series of steps, one can now invoke store.send and store.received directly, improving how/where XCTest failures are surfaced.

@stephencelis stephencelis requested a review from mbrandonw March 22, 2021 14:28
@stephencelis stephencelis merged commit f1faccc into main Mar 22, 2021
@stephencelis stephencelis deleted the assert-dsl-changes-2 branch March 22, 2021 17:27
@nmccann
Copy link
Contributor

nmccann commented Mar 22, 2021

Very minor (and I almost managed to comment on this before it was merged), but there are a number of usages of self.scheduler that could be simplified to scheduler. I could create a PR for that, assuming there wasn't a reason for keeping them that way.

@stephencelis
Copy link
Member Author

@nmccann We may be in the minority here, but we prefer to always be explicit about self even where it can be implicit. We think it makes it clearer when a value is a property vs. a local variable or constant, especially when reading outside of Xcode, like in PRs. We'd take submissions to add self. if we've missed it anywhere, though 😁

@nmccann
Copy link
Contributor

nmccann commented Mar 22, 2021

Fair enough - I'll gladly admit that it does help readability in PRs. Even if you're in the minority - it's your project and you raise some compelling points, so I'm happy to defer to your preferences :) I've long ago realized there is no "correct" choice in regards to such things - internal consistency within a project is the more important thing.

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.

4 participants