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

Document that eofline fix is not supported for single-line files. #2695

Merged
merged 1 commit into from
May 17, 2017

Conversation

mxl
Copy link
Contributor

@mxl mxl commented May 6, 2017

PR checklist

Overview of change:

I think we should clearly state that eofline fix is not supported for single-line files. It's not obvious without inspecting source code and looks like bug.

@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint, @mxl! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@mxl mxl force-pushed the eofline-warn-single-line-files branch from 5215a2f to 38331ee Compare May 6, 2017 13:33
@ajafff
Copy link
Contributor

ajafff commented May 6, 2017

@mxl There are several rules that don't fix all violations and none of them state that explicitly. For example prefer-const doesn't fix uninitialized variables, align doesn't fix if it would remove code, ....

IMO this shouldn't be included in description or FAILURE_STRING. If you really want to have this documented, I'd suggest adding it to Rule.metadata.descriptionDetails.

@mxl
Copy link
Contributor Author

mxl commented May 6, 2017

@ajafff That's not good if rule says that it can fix some violation but in certain cases it does not do this and that's not documented anywhere. I will add this to descriptionDetails. Any suggestions on how can I improve it?

@mxl mxl force-pushed the eofline-warn-single-line-files branch from 38331ee to 2b04e5a Compare May 7, 2017 13:49
@mxl mxl changed the title Warn that eofline fix is not supported for single-line files. Document that eofline fix is not supported for single-line files. May 7, 2017
Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

this looks fine; it's the best we can do for now; autofixing will never be perfect or address all scenarios

@adidahiya adidahiya merged commit 1160e35 into palantir:master May 17, 2017
@mxl mxl deleted the eofline-warn-single-line-files branch May 17, 2017 19:35
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.

4 participants