Skip to content

Need to conclude on precedence Settings files #518

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

Closed
3 tasks
joeyaiello opened this issue May 3, 2016 · 5 comments · Fixed by #614
Closed
3 tasks

Need to conclude on precedence Settings files #518

joeyaiello opened this issue May 3, 2016 · 5 comments · Fixed by #614

Comments

@joeyaiello
Copy link
Contributor

All up, we need to finish the design for Settings files. This has been discussed throughout a number of issues over the past year, but I'm having trouble tracking down the rest of the context (feel free to bring in other issues if you remember what they are).

Basically, we need to solve Settings file precedence, including well-known paths for where these Settings files are located. In my mind, there's 4 places from where a Settings file can be sourced, and the precedence of which one wins should be:

  1. The explicit Settings file called by the -Settings parameter.
  2. A Settings file in the root of the module folder with a well-known filename (this would be similar to a .gitignore file or Pester's use of a *.Tests.ps1 or a Tests\ folder).
  3. A user-wide Settings file (possibly stored in ~\AppData\Local or somewhere else under ~)
  4. A system-wide Settings file (possibly stored in $env:ProgramFiles\WindowsPowerShell\Modules\PSScriptAnalyzer)
  • Q1: are these all locations where Settings files should be stored?

  • Q2: is this the correct precedence?

  • Q3: For the module-scoped folder, what's the name? Some options we've thrown around:

  • FooModule.PSSA.psd1

  • FooModule.ScriptAnalyzer.psd1

  • FooModule.Settings.psd1

  • FooModule.PSScriptAnalyzer.psd1

  • FooModule.PssaSettings.psd1

  • FooModule.???.psd1

    Or is there something else we haven't thought about?

@daviwil
Copy link
Contributor

daviwil commented May 3, 2016

cc @rkeithhill since this has some impact on how we leverage PSScriptAnalyzer in Editor Services

@rkeithhill
Copy link
Contributor

rkeithhill commented May 5, 2016

Q1 - I don't see a strong need for a system-wide settings file. Even a per-user settings file is questionable. There is no precedent for this with FxCop or StyleCop. They don't have per-user settings files. I think the most common case (by far) will be project-specific settings file. Having settings be per-user is more brittle since you can't hand this source over to someone else and rely upon them having a settings file configured like yours. A project-specific settings file makes the project more portable especially when put on GitHub. So my priority order would be:

  • Per-project settings file specified explicitly - MUST
  • Per-project settings file used by default via standard name - Maybe but then you have to come up with a name. :-)
  • Per-user settings file - Probably not necessary.
  • Per-machine settings file - Not necessary.

And if I'm wrong you could add the last two later without breaking anything.

Q2. Yes that is the right order but I don't think you need a "defined, per-project settings file name". Just make the user specify the filename using the -Settings parameter. Pester is different IMO because you may have many pester test files and specifying them all is a bit of a pain. Here we are talking about just a single file. Plus this makes the answer to Q3 easy...

Q3. N/A if you require the user to specify the settings filename rather than "auto-finding" it. If I had to pick a name, I'd go with either ScriptAnalyzerSettings.psd1 or FooModule.ScriptAnalyzerSettings.psd1. Not sure what the benefit of the module name prefix is though?

Keep it simple and if people scream for more you can add most of these features without breaking changes.

@kapilmb
Copy link

kapilmb commented May 18, 2016

I agree that a per-project settings file lends more portability, however, I think we also need a system-wide or global settings file or something similar to that effect (like vscode default settings file). This file can also be used to feed the engine and the rules with default configuration data. For example, we can use the global settings file to add a default whitelist for PSAvoidUsingCmdletAliases, a whitelist for PSUseSingularNoun ( #479 ), etc. The user can then override these settings if he/she wants in the per-project settings file.

For a project-scoped setting file name, we can drop the project name.

@Jaykul
Copy link
Contributor

Jaykul commented Jul 11, 2016

I disagree with Keith on one point: I think a per-project file automatically discovered by name is THE WHOLE POINT of having a settings file. Anything else is icing on the cake.

Like fxcop, projects need to be able to define their own rules, and have new contributors use that set of rules by default without needing to use special command-lines (which tools like VSCode won't properly support anyway)! That is: it should be simple to produce a rule set and have Invoke-ScriptAnalyzer enforce those rules (and only those rules) when other people are running it.

For what it's worth, my Configuration module supports layering of settings: defaults + machine (+ enterprise) + user...

For settings, I think this is very useful. For turning on/off rules, I'm not so sure. I don't have any problem with a user settings file (or a machine settings file) that includes rule specifications if those files are only used when there's no local project file or when specified by hand, but layering rules is a problem. For example, from @kapilmb's post:

There's a default rule PSAvoidUsingCmdletAliases which I think is a bad rule so I would disable it in my projects and in my personal settings file, and I would have an alternate rule PSAvoidUsingPersonalAliases which would only complain about aliases that weren't built-in or defined within the script.

  1. I don't want other people to enable the default in their settings and override my project configuration and end up with a ton of "warnings" about my project -- which they then waste time fixing to send me a pull request I'm going to reject.
  2. On the other hand, other people don't want my settings to override the defaults when I'm contributing to their projects. If they think the defaults are perfect, they shouldn't have to explicitly turn on (and off) all the rules to get them to match the defaults just to prevent me from inadvertently submitting violations of PSAvoidUsingCmdletAliases, right?

Somewhere in this conversation I'm curious to hear what your thoughts are about whether these settings files have to just specify deviation from defaults, or whether there's a new-scriptanalyzerconfig command that pre-populates them with the current defaults such that they're impervious to future changes in the defaults.

@rkeithhill
Copy link
Contributor

@Jaykul I would be OK with load by filename convention as long as using the -Settings parameter overrides the convention. I might have more than one settings file. Rather than rename files, I'd prefer to use the non-primary settings file(s) by specifying -Settings and a path. Seems like this would be default behavior anyway.

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

Successfully merging a pull request may close this issue.

5 participants