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

[Breaking change] Remove createFix, and just use the Replacement(s) as the fix. (Minimal change) #2403

Merged
merged 1 commit into from
Mar 28, 2017

Conversation

andy-hanson
Copy link
Contributor

@andy-hanson andy-hanson commented Mar 25, 2017

PR checklist

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

Overview of change:

This is #2356, but made to have fewer changes to rule style.

CHANGELOG.md entry:

[api] Remove createFix. Replacements should be passed directly into addFailure.

Copy link
Contributor

@ajafff ajafff left a comment

Choose a reason for hiding this comment

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

LGTM

this really doesn't change the formatted output? I don't get how that works without changing the relevant code. Anyways, I'm ok with it if it just works

@adidahiya
Copy link
Contributor

sorry, this has conflicts now (likely because of #2359)

@andy-hanson
Copy link
Contributor Author

andy-hanson commented Mar 28, 2017

Turning tests on (#2406) revealed that it did change the output of the JSON formatter. But that should be fine, right?

@adidahiya adidahiya merged commit 4141de4 into palantir:master Mar 28, 2017
@ajafff
Copy link
Contributor

ajafff commented Mar 28, 2017

We should at least mention the changed (JSON) formatted output in the change log. It's hard to guess for embedders to guess the new format.
My suggestion:
[formatter-change] the `fix` property in `json` output now contains either one replacement or an array of replacements

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.

3 participants