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

Add option to ignore accessors in adjacent-overload-signatures #3718

Merged

Conversation

saberduck
Copy link
Contributor

@saberduck saberduck commented Feb 14, 2018

PR checklist

  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

This change adds option to ignore getters / setters when considering overloads in adjacent-overload-signatures to allow grouping getters and setters together.

CHANGELOG.md entry:

[new-rule-option] "ignore-accessors" for adjacent-overload-signatures to ignore getters / setters overloads

@saberduck
Copy link
Contributor Author

Because getters/setters are not overloads, I am thinking if this behavior should be configurable at all. Maybe the rule should be just changed to ignore getters/setters? If you agree I can remove this PR and create a new one

@giladgray
Copy link

@saberduck ah great point about them not being overloads. i actually like how the rule works currently, where it keeps get x() and set x() together. i suppose an option to disable that could be nice though.

@saberduck saberduck force-pushed the ignore_accessors_in_adjacent_overload branch from 1bd8bc5 to b8f8de9 Compare May 3, 2018 09:02
@saberduck
Copy link
Contributor Author

@giladgray I fixed the options schema, please have a look

@saberduck saberduck force-pushed the ignore_accessors_in_adjacent_overload branch from b8f8de9 to 9962297 Compare June 27, 2018 09:30
@saberduck
Copy link
Contributor Author

@giladgray can you please review again?

@saberduck
Copy link
Contributor Author

It seems that new precommit hook reformats tslint.json file and it differs from master, how can I avoid that?

@JoshuaKGoldberg
Copy link
Contributor

@saberduck yeah that problem's on TSLint's end; it's not a problem if your PR contains the changes. Prettier was enabled before everything was merged, so there are still a few files that need to be manually reformatted...

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Jun 16, 2019

Sorry for the delay @saberduck! I'm applying the formatting & merge changes now, and will merge this in once the build passes. It'll be available in the next TSLint release.

Just a heads up - I'm also changing the rule's options to be of the form { "ignore-accessors": true }. Other rules have had pains trying to mix and match string configurations with more complex option objects. It's been easier to stick with options objects like this.

@JoshuaKGoldberg JoshuaKGoldberg merged commit 20eec28 into palantir:master Jun 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants