Skip to content

RFC: Change Pipeline Indentation Style default from NoIndentation to IncreaseIndentationForFirstPipeline #4296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
2 tasks done
andyleejordan opened this issue Dec 1, 2022 · 2 comments
Labels

Comments

@andyleejordan
Copy link
Member

Prerequisites

  • I have written a descriptive issue title.
  • I have searched all issues to ensure it has not already been reported.

Summary

I've discovered that our default value to configure PSScriptAnalyzer's PipelineIndentation setting is NoIndentation, which differs from PSSA's own default (and arguably more popular) of IncreaseIndentationForFirstPipeline. This was last updated in May 2020 via #2698 which alludes to a bug in PSSA that I hope is since fixed.

@bergmeister in particular, what do you think?

Proposed Design

Just update the default in package.json to IncreaseIndentationForFirstPipeline. While I think this is a very desirable behavior, it would come at the cost of user-facing change that would affect formatting of scripts. So the question is, have the vast majority of people already overridden it with IncreaseIndentationForFirstPipeline?

Further RFC: Should we do a just an overhaul of all our default code formatting settings to match PSSA's defaults?

@andyleejordan andyleejordan added the Issue-Enhancement A feature request (enhancement). label Dec 1, 2022
@ghost ghost added the Needs: Triage Maintainer attention needed! label Dec 1, 2022
@andyleejordan andyleejordan added Issue-Discussion Let's talk about it. Area-Code Formatting and removed Issue-Enhancement A feature request (enhancement). Needs: Triage Maintainer attention needed! labels Dec 1, 2022
@JustinGrote
Copy link
Collaborator

The only issue I can think of is that people who have format-on-save set are gonna have a lot of suddenly large git commits and not understand why.

I think all the defaults to match PSSA defaults should be done all at once as a single breaking change with a popup dialog at start letting people know it has changed and give them a button to go back, maybe have an overarching "classic" setting to allow people to bypass that, rather than onesy-twosy.

@bergmeister
Copy link
Contributor

bergmeister commented Dec 2, 2022

This needs historic background:
When I first added this feature and setting, IncreaseIndentationForFirstPipeline was actually the default but it turned out that even after a couple iterations there were still some edge cases. Therefore I added this NoIndentation option later, which kind of acts like a feature flag to provide the behaviour it had before I added the feature to reduce the blast radius and give me more time to work on fixing edge cases.
It's hard to say whether the feature is now good enough for everyone to be on by default but with the preview extension we could try temporarily? Would it help if I look up existing issues of this setting to help make a decision?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants