DSC Community guidelines instructions

[johlju]

Pull Request (PR) description

WIP

This Pull Request (PR) fixes the following issues

Task list

• Added an entry to the change log under the Unreleased section of the
  file CHANGELOG.md. Entry should say what was changed and how that
  affects users (if applicable), and reference the issue being resolved
  (if applicable).
• Resource documentation updated in the resource's README.md.
• Resource parameter descriptions updated in schema.mof.
• Comment-based help updated, including parameter descriptions.
• Localization strings updated.
• Examples updated.
• Unit tests updated. See DSC Community Testing Guidelines.
• Integration tests updated (where possible). See DSC Community Testing Guidelines.
• Code changes adheres to DSC Community Style Guidelines.

---

This change is 

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (6)

.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md (2)

3-3: Scope the applyTo glob to command integration tests only

Limit the rule to tests under tests/Integration/Commands to avoid unintended matches.

-applyTo: "**/*.[iI]ntegration.[tT]ests.ps1"
+applyTo: "tests/[Ii]ntegration/[Cc]ommands/**/*.[Ii]ntegration.[Tt]ests.ps1"

---

25-27: Wrap long lines in code fence (MD013) and improve readability

Split the long attribute and the thrown error string without changing behavior.

-[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')]
+[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute(
+    'PSUseDeclaredVarsMoreThanAssignments',
+    '',
+    Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.'
+)]

-        throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.'
+        throw 'DscResource.Test module dependency not found. Please run ' +
+              '".\build.ps1 -ResolveDependency -Tasks build" first.'

Also applies to: 46-46

.github/instructions/SqlServerDsc-guidelines.instructions.md (1)

93-93: Tighten phrasing and add article; wrap to avoid MD013

Clarifies grammar and readability.

-When using command `Connect-SqlDscDatabaseEngine` always use `Disconnect-SqlDscDatabaseEngine` when connection is no longer needed.
+When using `Connect-SqlDscDatabaseEngine`, always use
+`Disconnect-SqlDscDatabaseEngine` when the connection is no longer needed.

.github/instructions/dsc-community-style-guidelines-powershell.instructions.md (3)

149-151: Replace non-standard Get-ComputerName with cross-platform alternative

Get-ComputerName is not a built-in, cross-platform cmdlet. Recommend a reliable API.

-- Avoid hardcoded computer names, use cross-platform [`Get-ComputerName`](https://github.com/dsccommunity/DscResource.Common/wiki/Get%E2%80%91ComputerName) instead of `$env:COMPUTERNAME`
+- Avoid hardcoded computer names. For a cross-platform machine name, use
+  `[System.Environment]::MachineName` instead of `$env:COMPUTERNAME`.

---

152-153: Do not recommend SilentlyContinue; prefer explicit error handling

SilentlyContinue can hide real issues; show a scoped try/catch pattern instead.

-- Avoid empty catch blocks (use `-ErrorAction SilentlyContinue`)
+- Avoid empty catch blocks. Prefer:
+  `try { } catch [SpecificException] { <handle/log/rethrow> }`.
+  Avoid `-ErrorAction SilentlyContinue` unless explicitly justified and safe.

---

159-159: Clarify guidance about one-time variables

Make the intent explicit and actionable.

-- Use declared variables more than once
+- Avoid declaring throwaway variables used only once; inline expressions
+  when it improves readability, otherwise prefer well‑named variables.

[coderabbitai]

Actionable comments posted: 8

🔭 Outside diff range comments (1)

.github/instructions/dsc-community-style-guidelines-changelog.instructions.md (1)

14-14: Stray line artifact at Line 14.

Looks like an orphaned line number/formatting artifact; remove it.

-14
+ 

♻️ Duplicate comments (23)

.github/instructions/dsc-community-style-guidelines-changelog.instructions.md (1)

8-13: Tighten wording, fix grammar, and capitalize “Keep a Changelog.”

Concise, prescriptive bullets; fixes pluralization and avoids duplicates.

-- Always update the Unreleased section in CHANGELOG.md
-- Use Keep a Changelog format
-- Describe main changes as concise release notes
-- Reference issues using format #<issue_number>
-- No empty lines between list items in same section
-- Do not add item if there are already an existing item for the same change
+- Always update the Unreleased section in CHANGELOG.md with every change.
+- Use the Keep a Changelog format.
+- Provide concise release notes that describe the main changes.
+- Reference related issues using the format #<issue_number>.
+- Do not leave empty lines between list items in the same section.
+- Do not add a new item if an existing item already covers the same change.

.github/instructions/dsc-community-style-guidelines-integration-tests.instructions.md (5)

3-3: Scope applyTo to command integration tests only.

This doc targets command integration tests; narrow the glob to avoid unintended matches.

-applyTo: "tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1"
+applyTo: "tests/[Ii]ntegration/[Cc]ommands/**/*.[Ii]ntegration.[Tt]ests.ps1"

---

13-14: Clarify CI-only helper and where to get the computer name locally.

Readers outside CI won’t know about Get-ComputerName.

-- Use `Get-ComputerName` for computer names in CI
-- Only run integration tests in CI unless explicitly instructed.
+- Use `Get-ComputerName` for computer names in CI. This helper is provided
+  by the build pipeline and is not required locally (use $env:COMPUTERNAME
+  or `Get-CimInstance -ClassName Win32_ComputerSystem` when running locally).
+- Run integration tests only in CI unless explicitly instructed otherwise.

---

18-21: Wrap long attribute line and add a blank line before the fence (MD022/MD031).

Improves readability and satisfies markdownlint.

-```powershell
-[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')]
-param ()
+```powershell
+[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute(
+    'PSUseDeclaredVarsMoreThanAssignments',
+    '',
+    Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.'
+)]
+param ()

---

40-41: Split long throw message to satisfy MD013 (80-char) rule.

-        throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.'
+        throw 'DscResource.Test module dependency not found. Please run ' +
+              '".\build.ps1 -ResolveDependency -Tasks build" first.'

---

21-23: Minor language and import robustness.

• Prefer “before” vs “prior to”; tighten phrasing.
• Include -Force and -ErrorAction Stop on Import-Module (second instance already OK).

-## Required Setup Block
+## Required Setup Block
@@
-BeforeAll {
+BeforeAll {
     $script:dscModuleName = 'SqlServerDsc'
 
-    Import-Module -Name $script:dscModuleName -Force -ErrorAction 'Stop'
+    Import-Module -Name $script:dscModuleName -Force -ErrorAction 'Stop'
 }

Note: Wording above the code fence already uses “Required Setup Block”; no change needed there.

Also applies to: 44-48

.github/instructions/dsc-community-style-guidelines-localization.instructions.md (5)

13-16: Clarify file locations and wrap paths in code spans.

Explicit and consistent with DSC Community practices.

-## String Files
-- Commands/functions: `source/en-US/SqlServerDsc.strings.psd1`
-- Class resources: `source/en-US/{ResourceClassName}.strings.psd1`
+## String Files
+
+- Public commands and private functions:
+  `source/en-US/SqlServerDsc.strings.psd1`
+- Class-based resources:
+  `source/en-US/<ResourceClassName>.strings.psd1`

---

17-20: State naming rules explicitly and give a realistic example.

Aligns with established key naming (function/resource-prefixed, underscores).

-## Key Naming
-- Format: `FunctionName_Description` (underscore separators)
-- Example: `Get_SqlDscDatabase_ConnectingToDatabase`
+## String Key Naming
+
+- Prefix keys with the function or resource name using underscores.
+- Use underscores as word separators for multiword keys.
+- Example: `Get_SqlDscSqlDatabase_OperationFailed`

---

21-26: Add blank line before fence and align example key and ID.

Improves MD031 and demonstrates the convention.

-## String Format
-```powershell
+## String Format
+
+```powershell
 ConvertFrom-StringData @'
-    KeyName = Message with {0} placeholder. (PREFIX0001)
+    Get_SqlDscSqlDatabase_OperationFailed = Message with {0} placeholder. (GSDSD0001)
 '@

---

`28-32`: **Expand ID rules, fix phrasing, and break long examples into bullets.**

Clarifies “starting at 0001” and avoids line-length issues.


```diff
-## String IDs
-- Format: `(PREFIX####)`
-- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
-- Number: Sequential from 0001
+## String ID Format
+
+- Use unique IDs: `(PREFIX####)`
+- PREFIX: First letter of each word in the resource or function name.
+- Number: Sequential, starting at 0001.
+- Examples:
+  - SqlSetup → SS0001
+  - SqlAGDatabase → SAGD0001
+  - Get-SqlDscSqlDatabase → GSDSD0001

---

33-36: Shorten long lines in usage examples and add a blank line before fence.

-## Usage
-```powershell
-Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
-```
+## Usage
+
+```powershell
+# Verbose/Warning
+Write-Verbose -Message ($script:localizedData.KeyName -f $value)
+Write-Warning -Message ($script:localizedData.KeyName -f $value)
+
+# Error
+$msg = $script:localizedData.KeyName -f $value1, $value2
+New-InvalidOperationException -Message $msg
+
+$msg = $script:localizedData.KeyName -f $value1
+New-InvalidOperationException -ErrorRecord $_ -Message $msg
+```

.github/instructions/SqlServerDsc-guidelines.instructions.md (2)

29-32: Reflow pipeline guidance and name the stage for discoverability.

-- Integration test script files must be added to a group
-within the test stage in ./azure-pipelines.yml.
-- Choose the appropriate group number based on the required dependencies
+- Integration test script files for public commands must be added to a group
+  within the `Integration_Test_Commands_SqlServer` stage in
+  `./azure-pipelines.yml`. Choose the appropriate group number based on the
+  command’s dependencies (for example, commands that require Database Engine
+  should be in Group 2 or later, after the Database Engine installation tests).

---

11-14: Clarify inheritance guidance and list punctuation.

Clearer modality and proper commas in the provided members.

-## Resources
-- Database Engine resources: inherit `SqlResourceBase`
-- `SqlResourceBase` provides: `InstanceName`, `ServerName`, `Credential`, `Reasons`, `GetServerObject()`
+## Resources
+
+- Database Engine resources should inherit from `SqlResourceBase`
+  (which itself inherits `ResourceBase`).
+- `SqlResourceBase` provides the DSC properties `InstanceName`,
+  `ServerName`, `Credential`, and `Reasons`, and the `GetServerObject`
+  method used to connect to a SQL Server Database Engine instance.

.github/instructions/dsc-community-style-guidelines-class-resource.instructions.md (2)

62-66: Grammar: fix NormalizeProperties comment (subject–verb agreement, clarity).

Same issues as above.
[ suggest_nitpick ]

 hidden [void] NormalizeProperties([System.Collections.Hashtable] $properties)
 {
-    # Normalize user-provided properties
-    # Variable $properties contains properties user assigned values.
+    # Normalize user-provided properties.
+    # The $properties variable contains the properties that the user assigned values to.
 }

---

44-48: Clarity: expand GetCurrentState guidance and article usage.

Add “a hashtable” and clarify the role of $properties.
[ suggest_nitpick ]

 hidden [System.Collections.Hashtable] GetCurrentState([System.Collections.Hashtable] $properties)
 {
-    # Return current state as hashtable
-    # Variable $properties contains the key properties (key-value pairs).
+    # Return the current state as a hashtable.
+    # The $properties variable contains the key properties (key-value pairs).
 }

.github/instructions/dsc-community-style-guidelines-powershell.instructions.md (5)

151-151: Fix: recommend cross‑platform machine name API instead of non-existent Get-ComputerName.

Use a standard .NET API; link can remain as an alternative helper if desired.
[ suggest_essential_refactor ]

-- Avoid hardcoded computer names, use cross-platform [`Get-ComputerName`](https://github.com/dsccommunity/DscResource.Common/wiki/Get%E2%80%91ComputerName) instead of `$env:COMPUTERNAME`
+- Avoid hardcoded computer names. For a cross‑platform machine name, use
+  `[System.Environment]::MachineName` instead of `$env:COMPUTERNAME`.

---

153-155: Do not recommend SilentlyContinue in lieu of proper error handling.

SilentlyContinue can hide real issues. Prefer explicit try/catch with specific exception types.
[ suggest_essential_refactor ]

-- Avoid empty catch blocks (use `-ErrorAction SilentlyContinue`)
+- Avoid empty catch blocks. Prefer:
+  `try { <call> } catch [SpecificException] { <handle/log/rethrow> }`.
+  Avoid `-ErrorAction SilentlyContinue` unless narrowly scoped and justified.

---

36-36: Modal verb/line-wrap: “variable expansion is needed.”

Minor grammar and wrapping to avoid future MD013.
[ suggest_nitpick ]

-- Use single quotes unless variable expansion is needed: `'text'` vs `"text $variable"`
+- Use single quotes unless variable expansion is needed:
+  `'text'` vs `"text $variable"`.

---

42-43: Clarify “unary comma” guidance for arrays.

Name the operator explicitly and suggest preferred alternatives.
[ suggest_nitpick ]

-- Do not use the unary comma operator (`,`) in return statements to force
-  an array
+- Do not use the unary comma operator (`,`) in return statements to force an array;
+  prefer `@()` or a consistent output type.

---

146-147: Clarify “one object” guidance: type consistency matters.

Recommend consistent object types rather than a single instance.
[ suggest_nitpick ]

-- Return a single, consistent object type per function
+- Return a single, consistent object type per function (avoid mixing
+  scalars, collections, or different types in one pipeline).

.github/instructions/dsc-community-style-guidelines-pester.instructions.md (1)

29-29: Article usage: “the prefix 'mock'.”

Minor grammar fix.
[ suggest_nitpick ]

-- Mock variables prefix: 'mock'
+- Mock variables prefix: 'mock' (use the prefix 'mock')

.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md (2)

18-20: Remove duplicate SuppressMessageAttribute; keep one with justification and wrap.

Two identical suppressions are present; retain a single, wrapped attribute.
[ suggest_essential_refactor ]

-[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')]
-[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')]
+[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute(
+    'PSUseDeclaredVarsMoreThanAssignments',
+    '',
+    Justification = 'Pester discovery assigns variables in a way ScriptAnalyzer misinterprets.'
+)]

---

40-41: Split long thrown error string to respect line length (MD013).

Non-functional reflow to avoid future lint noise.
[ suggest_nitpick ]

-        throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.'
+        throw 'DscResource.Test module dependency not found. Please run ' +
+              '".\build.ps1 -ResolveDependency -Tasks build" first.'