Join the Stream and Verbose flags into the verbosity value #73
Conversation
If the runner needs the Stream value from the config, it should not get it from the reporter. It is better to keep a copy of the value in the runner itself to make the communication between runner and reporter only one-direction.
|
I reverted the change to the RunConf type. |
|
|
||
| var verbosity uint8 | ||
| if conf.Verbose { | ||
| verbosity = 1 |
There was a problem hiding this comment.
I would use a type here for the enumerated constant values, take a look at https://golang.org/doc/effective_go.html#constants
There was a problem hiding this comment.
I used a number instead of constants because we don't have names for the different levels.
We now could use lowVerbosity and highVerbosity. But we already need three levels of verbosity for the snappy tests, and we will probably add an extra level at some point.
|
@ElOpio looks good, minor comment :) |
|
Stream means do not cache output.. Verbose means show more output than usual. This is significantly more clear than Verbose being 1 or 2. Can we please focus on actual improvements? |
|
@niemeyer that is the problem. You are using the log as a cache for the reporter, sometimes, depending on the flags. In #75 I split them, as the log should have its own configuration separate from the reporter. And if you want the log to also go to stdout, or to somewhere else, we can add that. |
How is that a problem?
How is it not true? |
|
Also, it would be nice if before closing the PRs, we had a discussion about them. I'm explaining the rationale for all my changes, and why they are needed to finally implement the subunit reporter that's all I want. You are here not offering alternatives, just saying that the work I did improves in no way your project, rejecting it before I can even reply. And if that's the case, if this is no improvement at all, give me a hand and offer some suggestions. |
It would be even better if we had a conversation before opening a PR. |
|
We had a conversation, and you told me to implement the interface in a correct way that could be moved to a separate package. Here it is, #76. Split in tiny progressive improvements for having a pleasant discussion on each of them. How is it a problem that the log and the reporter are too coupled? It prevents us from writing an independent reporter. Also from writing an independent log, which would be nice too have too. How is it not true that stream mean cache? Because "The only way to use it now is to pass -vv, which means more verbose than -v." So stream means -vv. |
No, we didn't Leo. This PR proposes a change we never talked about.
Stream and Verbose is part of the implementation and of a public API. Stream means "stream results out as soon as you get them", Verbose means "tell me more about what you are doing". This both how it is internally implemented, and exposed in the public-facing API of the package. The fact -vv sets both is irrelevant, and does not change any of those facts.
Please leave the implementation with me Leo. We're wasting our time talking about two booleans instead of making any progress at all on the actual reporter interface. |
|
as you wish. Please give me an ETA for when we will have the subunit reporter, so I can tell the certification team when to expect it. |
The word Stream is confusing. It has many meanings, and it's not clear that it means more verbose than verbose. In addition to that, using a numerical value allows future runners and reporters to have a mode that's even more verbose than Stream.
Requires #71.