Skip to content

Conversation

kkozmic
Copy link
Contributor

@kkozmic kkozmic commented Aug 18, 2013

I'm (almost) happy with this one. There's still some work to be done around formatters, but I'm quite happy with how the rest of that turned out.

IMO it's much neater and more flexible than the first few attempts we've had.

kkozmic added 11 commits August 18, 2013 11:30
This seems like a better abstraction, especially that some of them actually not render anything but approve/validate the results.
This seems like a better abstraction not imposing any particular way the result should be handled
and into ConventionContext
They should be in control of how they want the data formatted. We still need to work on how to actually put the two together...
ConventionResult now holds formatted result and recommended file extension. This will save us from having to have a copy of text renderer in almost any other renderer.
Also this gives us flexibility to either use existing result, or overwrite it (or ignore and render your own, but not save it back for validation, like HTML renderer does
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have lost the reporting of the second convention failing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope. It's split into a second file now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@JakeGinnivan
Copy link
Member

Im happy to merge, the other stuff can be fixed in another PR

JakeGinnivan pushed a commit that referenced this pull request Aug 18, 2013
@JakeGinnivan JakeGinnivan merged commit 23dd61b into TestStack:master Aug 18, 2013
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the second file :)

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