GeezerVison

Thanks Matěj Kafka (@MatejKafka)! I really like this. Could I ask you to do one small refactor? I checked and RunAnalysis() is returning an IEnumerable which is why you then called ToArray() (well, you could have also used LINQ's Count() extension method, but that's what we're trying to avoid). Instead, since the container of diagnostic records being returned is already a List could you change the return type of RunAnalysis to List? Then you can use the Count property instead of method, and continue to pass it to the next function that takes an IEnumerable (which List satisfies and won't cause any type changes) instead of changing it to take an array.

Andy Jordan (@andyleejordan) Needed to make a few more changes in AnalyzeScriptDefinition, which returned an IEnumerable, which is why did not make this change in the first place, but agree it's the better option. :)

Btw, looking at the code a bit more, imo it would also make sense to move ReportSummary handling to EndProcessing – that way, it's consistent with the new EnableExit behavior, and I'd assume most users want the summary for the whole run, not for each piped file separately. Thoughts?

Btw, looking at the code a bit more, imo it would also make sense to move ReportSummary handling to EndProcessing – that way, it's consistent with the new EnableExit behavior, and I'd assume most users want the summary for the whole run, not for each piped file separately. Thoughts?

Christoph Bergmeister (@bergmeister) thoughts on this? I'm inclined to agree but it seems like a riskier change. Otherwise I'm good with this PR as-is but wanted to ping you before merging.

Matěj Kafka (@MatejKafka) I chatted with Patrick Meinecke (@SeeminglyScience) and confirmed that wouldn't break things, so I'm all for adding that small refactor. It makes sense, then I can do a final review and merge this.

One more thing – currently, the number of violations reported by -ReportSummary does not include parse errors, which is imo a bit weird, especially when the exit code includes them. Would you agree to also including parse errors in the total number of violations and the "Error = ..." print to be consistent?

Updated with changes to -ReportSummary. I tend to make a lot of minor cleanup changes while editing code. If you're not happy about any of the changes, just ping me and I'll remove them from the PR. :)

Ah, yes, sorry. I misunderstood and that moving the report summary logic would be a change on the same order as the exit code change. Need to keep it very minimal.

Not sure I follow – are you referring to changing -ReportExit at all, or the extra changes on top of it?

All the extra changes on top of it. I'm good to merge just commit 04a0149. For the extra report summary changes proposed, let's do it another PR.

Makes sense, I'll move the follow-up commits to separate PRs, thanks. :)