Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

vsoFormatter: don't duplicate output for fixed failures #3348

Merged
merged 2 commits into from Oct 20, 2017
Merged

vsoFormatter: don't duplicate output for fixed failures #3348

merged 2 commits into from Oct 20, 2017

Conversation

ajafff
Copy link
Contributor

@ajafff ajafff commented Oct 19, 2017

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

[bugfix] vso formatter no longer duplicates output for fixed failures

I noticed this while looking through the formatters to find out how the fixes parameter is used. Obviously this formatter got it wrong...

Is there anything you'd like reviewers to focus on?

CHANGELOG.md entry:

[bugfix] `vso` formatter no longer duplicates output for fixed failures
@@ -45,7 +45,7 @@ describe("VSO Formatter", () => {
getFailureString(TEST_FILE, 2, 12, "mid failure", "mid-name") +
getFailureString(TEST_FILE, 9, 2, "last failure", "last-name");

const actualResult = formatter.format(failures);
const actualResult = formatter.format(failures, failures);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is confusing to read -- the second argument is fixes but it's called failures here. you might want to write a separate test to validate that passing in fixes as a second argument doesn't create excess formatter output, or at least leave a comment here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a separate test where I duplicate the failures array to pass them as fixes

@adidahiya adidahiya merged commit 272f66e into palantir:master Oct 20, 2017
HyphnKnight pushed a commit to HyphnKnight/tslint that referenced this pull request Apr 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants