Skip to content

Conversation

@raandree
Copy link
Collaborator

@raandree raandree commented Sep 22, 2025

Pull Request

Pull Request (PR) description

Added

  • Add WindowsFeatureSourcePath parameter to OfficeOnlineServerSetup for installing removed feature .net Framework 3.5

Task list

  • The PR represents a single logical change. i.e. Cosmetic updates should go in different PRs.
  • Added an entry under the Unreleased section of in the CHANGELOG.md as per format.
  • Local clean build passes without issue or fail tests (build.ps1 -ResolveDependency).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is Reviewable

@coderabbitai
Copy link

coderabbitai bot commented Sep 22, 2025

Walkthrough

Adds a mandatory WindowsFeatureSourcePath parameter to OfficeOnlineServerSetup, introduces a WindowsFeature resource NetFx35 to install Net-Framework-Core from that Source, removes NET-Framework-Core from xWindowsFeatureSet, updates WMIPerformanceAdapter DependsOn, updates unit test config, and updates CHANGELOG.

Changes

Cohort / File(s) Change summary
DSC resource schema updates
source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1
Added mandatory parameter WindowsFeatureSourcePath. Added [WindowsFeature] NetFx35 (Name=Net-Framework-Core, Ensure=Present, Source bound to WindowsFeatureSourcePath). Removed 'NET-Framework-Core' from xWindowsFeatureSet. Updated [xService] WMIPerformanceAdapter DependsOn to include ,[WindowsFeature]NetFx35.
Tests configuration
tests/Unit/DSCResources/Assets/Config/OfficeOnlineServerSetup.yml
Added top-level WindowsFeatureSourcePath: \\server\Data\sxs. Removed Language: en-us entry under LanguagePacks. BinaryDir entries unchanged.
Documentation
CHANGELOG.md
Added Unreleased entry describing WindowsFeatureSourcePath parameter for installing the removed .NET Framework 3.5 feature.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Admin
    participant LCM as DSC LCM
    participant Config as OfficeOnlineServerSetup
    participant NetFx as WindowsFeature NetFx35
    participant XWF as xWindowsFeatureSet OfficeOnlineServer
    participant Svc as xService WMIPerformanceAdapter

    Admin->>LCM: Apply configuration
    LCM->>Config: Compile with Path + WindowsFeatureSourcePath
    Note over Config: Declares NetFx35, OfficeOnlineServer features, and service with DependsOn

    rect rgba(220,240,255,0.4)
    LCM->>NetFx: Ensure Net-Framework-Core Present\nSource = WindowsFeatureSourcePath
    NetFx-->>LCM: Net-Framework-Core installed
    end

    LCM->>XWF: Install OfficeOnlineServer feature set\n(excludes NET-Framework-Core)
    XWF-->>LCM: Features installed

    rect rgba(235,255,235,0.5)
    LCM->>Svc: Configure WMIPerformanceAdapter\nDependsOn: NetFx35, xWindowsFeatureSet
    Svc-->>LCM: Service configured
    end

    LCM-->>Admin: Configuration applied
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Added WindowsFeatureSourcePath parameter to OfficeOnlineServerSetup" succinctly and accurately describes the primary change — adding the WindowsFeatureSourcePath parameter to the OfficeOnlineServerSetup resource — and is clear and specific enough for a reviewer scanning history.
Description Check ✅ Passed The PR description directly states the addition of the WindowsFeatureSourcePath parameter, includes verification steps (build/tests), and lists outstanding documentation and test tasks, so it is clearly related to the changeset and passes this lenient description check.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68d67cf and 6f0b153.

📒 Files selected for processing (1)
  • source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1 (3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**

⚙️ CodeRabbit configuration file

**: # DSC Community Guidelines

Terminology

  • Command: Public command
  • Function: Private function
  • Resource: DSC class-based resource

Build & Test Workflow Requirements

  • Run PowerShell script files from repository root
  • Setup build and test environment (once per pwsh session): ./build.ps1 -Tasks noop
  • Build project before running tests: ./build.ps1 -Tasks build
  • Always run tests in new pwsh session: Invoke-Pester -Path @({test paths}) -Output Detailed

File Organization

  • Public commands: source/Public/{CommandName}.ps1
  • Private functions: source/Private/{FunctionName}.ps1
  • Unit tests: tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1
  • Integration tests: tests/Integration/Commands/{CommandName}.Integration.Tests.ps1

Requirements

  • Follow instructions over existing code patterns
  • Follow PowerShell style and test guideline instructions strictly
  • Always update CHANGELOG.md Unreleased section
  • Localize all strings using string keys; remove any orphaned string keys
  • Check DscResource.Common before creating private functions
  • Separate reusable logic into private functions
  • DSC resources should always be created as class-based resources
  • Add unit tests for all commands/functions/resources
  • Add integration tests for all public commands and resources

Files:

  • source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1
{**/*.ps1,**/*.psm1,**/*.psd1}

⚙️ CodeRabbit configuration file

{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell Guidelines

Naming

  • Use descriptive names (3+ characters, no abbreviations)
  • Functions: PascalCase with Verb-Noun format using approved verbs
  • Parameters: PascalCase
  • Variables: camelCase
  • Keywords: lower-case
  • Classes: PascalCase
  • Include scope for script/global/environment variables: $script:, $global:, $env:

File naming

  • Class files: ###.ClassName.ps1 format (e.g. 001.SqlReason.ps1, 004.StartupParameters.ps1)

Formatting

Indentation & Spacing

  • Use 4 spaces (no tabs)
  • One space around operators: $a = 1 + 2
  • One space between type and variable: [String] $name
  • One space between keyword and parenthesis: if ($condition)
  • No spaces on empty lines
  • Try to limit lines to 120 characters

Braces

  • Newline before opening brace (except variable assignments)
  • One newline after opening brace
  • Two newlines after closing brace (one if followed by another brace or continuation)

Quotes

  • Use single quotes unless variable expansion is needed: 'text' vs "text $variable"

Arrays

  • Single line: @('one', 'two', 'three')
  • Multi-line: each element on separate line with proper indentation
  • Do not use the unary comma operator (,) in return statements to force
    an array

Hashtables

  • Empty: @{}
  • Each property on separate line with proper indentation
  • Properties: Use PascalCase

Comments

  • Single line: # Comment (capitalized, on own line)
  • Multi-line: <# Comment #> format (opening and closing brackets on own line), and indent text
  • No commented-out code

Comment-based help

  • Always add comment-based help to all functions and scripts
  • Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
  • Comment-based help indentation: keywords 4 spaces, text 8 spaces
  • Include examples for all parameter sets and combinations
  • INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...

Files:

  • source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1
source/DSCResources/**/*.psm1

⚙️ CodeRabbit configuration file

source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources Guidelines

Required Functions

  • Every DSC resource must define: Get-TargetResource, Set-TargetResource, Test-TargetResource
  • Export using *-TargetResource pattern

Function Return Types

  • Get-TargetResource: Must return hashtable with all resource properties
  • Test-TargetResource: Must return boolean ($true/$false)
  • Set-TargetResource: Must not return anything (void)

Parameter Guidelines

  • Get-TargetResource: Only include parameters needed to retrieve actual current state values
  • Get-TargetResource: Remove non-mandatory parameters that are never used
  • Set-TargetResource and Test-TargetResource: Must have identical parameters
  • Set-TargetResource and Test-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help comment

Required Elements

  • Each function must include Write-Verbose at least once
    • Get-TargetResource: Use verbose message starting with "Getting the current state of..."
    • Set-TargetResource: Use verbose message starting with "Setting the desired state of..."
    • Test-TargetResource: Use verbose message starting with "Determining the current state of..."
  • Use localized strings for all messages (Write-Verbose, Write-Error, etc.)
  • Import localized strings using Get-LocalizedData at module top

Error Handling

Files:

  • source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1
🔇 Additional comments (2)
source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1 (2)

57-62: Ensure .NET 3.5 features install from SxS media: guard Source, move 3.5 WCF features out of the set, and update DependsOn

xWindowsFeatureSet doesn’t support Source. The 3.5 components (NET‑HTTP‑Activation, NET‑Non‑HTTP‑Activ, and the meta‑feature NET‑Framework‑Features) often require the SxS source; leaving them in the set risks install failures on media‑less images. Also, emitting Source = $null can be avoided by guarding the NetFx35 block.

Guard the NetFx35 resource so Source is only set when provided (replace Lines 57–62):

-    WindowsFeature NetFx35
-    {
-        Name   = 'Net-Framework-Core'
-        Ensure = 'Present'
-        Source = $WindowsFeatureSourcePath
-    }
+    if ($WindowsFeatureSourcePath)
+    {
+        WindowsFeature NetFx35
+        {
+            Name   = 'Net-Framework-Core'
+            Ensure = 'Present'
+            Source = $WindowsFeatureSourcePath
+        }
+    }
+    else
+    {
+        WindowsFeature NetFx35
+        {
+            Name   = 'Net-Framework-Core'
+            Ensure = 'Present'
+        }
+    }

Remove 3.5 WCF entries from the feature set (Lines 87–93):

-            'NET-Framework-Features',
-            'NET-HTTP-Activation',
-            'NET-Non-HTTP-Activ',
+            # 3.5 features managed via dedicated WindowsFeature blocks with Source

Add dedicated resources for the 3.5 WCF features (place after the NetFx35 block):

WindowsFeature NetFx35HttpActivation
{
    Name   = 'NET-HTTP-Activation'
    Ensure = 'Present'
    Source = $WindowsFeatureSourcePath
}

WindowsFeature NetFx35NonHttpActivation
{
    Name   = 'NET-Non-HTTP-Activ'
    Ensure = 'Present'
    Source = $WindowsFeatureSourcePath
}

Update the WMI service dependency (Line 98):

-        DependsOn   = '[xWindowsFeatureSet]OfficeOnlineServer', '[WindowsFeature]NetFx35'
+        DependsOn   = '[xWindowsFeatureSet]OfficeOnlineServer', '[WindowsFeature]NetFx35', '[WindowsFeature]NetFx35HttpActivation', '[WindowsFeature]NetFx35NonHttpActivation'

Also applies to: 87-93, 98-102


38-40: Use full .NET type for the new parameter

Per the PowerShell guidelines, parameters should use full type names.

-        [string]
+        [System.String]
         $WindowsFeatureSourcePath

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
CHANGELOG.md (1)

29-30: Fix casing/formatting; optionally reclassify as "Added".

Use .NET proper casing and our markdown text rules (parameters bold; resource names italic). Optionally move this bullet under an “Added” subsection for Unreleased.

-- Add `WindowsFeatureSourcePath` parameter to `OfficeOnlineServerSetup` for
-  installing removed feature .net Framework 3.5.
+- Add **WindowsFeatureSourcePath** parameter to _OfficeOnlineServerSetup_ to
+  enable installing the removed .NET Framework 3.5 feature.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a24ddf and 294d78e.

📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1 (3 hunks)
  • tests/Unit/DSCResources/Assets/Config/OfficeOnlineServerSetup.yml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**

⚙️ CodeRabbit configuration file

**: # DSC Community Guidelines

Terminology

  • Command: Public command
  • Function: Private function
  • Resource: DSC class-based resource

Build & Test Workflow Requirements

  • Run PowerShell script files from repository root
  • Setup build and test environment (once per pwsh session): ./build.ps1 -Tasks noop
  • Build project before running tests: ./build.ps1 -Tasks build
  • Always run tests in new pwsh session: Invoke-Pester -Path @({test paths}) -Output Detailed

File Organization

  • Public commands: source/Public/{CommandName}.ps1
  • Private functions: source/Private/{FunctionName}.ps1
  • Unit tests: tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1
  • Integration tests: tests/Integration/Commands/{CommandName}.Integration.Tests.ps1

Requirements

  • Follow instructions over existing code patterns
  • Follow PowerShell style and test guideline instructions strictly
  • Always update CHANGELOG.md Unreleased section
  • Localize all strings using string keys; remove any orphaned string keys
  • Check DscResource.Common before creating private functions
  • Separate reusable logic into private functions
  • DSC resources should always be created as class-based resources
  • Add unit tests for all commands/functions/resources
  • Add integration tests for all public commands and resources

Files:

  • tests/Unit/DSCResources/Assets/Config/OfficeOnlineServerSetup.yml
  • source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1
  • CHANGELOG.md
{**/*.ps1,**/*.psm1,**/*.psd1}

⚙️ CodeRabbit configuration file

{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell Guidelines

Naming

  • Use descriptive names (3+ characters, no abbreviations)
  • Functions: PascalCase with Verb-Noun format using approved verbs
  • Parameters: PascalCase
  • Variables: camelCase
  • Keywords: lower-case
  • Classes: PascalCase
  • Include scope for script/global/environment variables: $script:, $global:, $env:

File naming

  • Class files: ###.ClassName.ps1 format (e.g. 001.SqlReason.ps1, 004.StartupParameters.ps1)

Formatting

Indentation & Spacing

  • Use 4 spaces (no tabs)
  • One space around operators: $a = 1 + 2
  • One space between type and variable: [String] $name
  • One space between keyword and parenthesis: if ($condition)
  • No spaces on empty lines
  • Try to limit lines to 120 characters

Braces

  • Newline before opening brace (except variable assignments)
  • One newline after opening brace
  • Two newlines after closing brace (one if followed by another brace or continuation)

Quotes

  • Use single quotes unless variable expansion is needed: 'text' vs "text $variable"

Arrays

  • Single line: @('one', 'two', 'three')
  • Multi-line: each element on separate line with proper indentation
  • Do not use the unary comma operator (,) in return statements to force
    an array

Hashtables

  • Empty: @{}
  • Each property on separate line with proper indentation
  • Properties: Use PascalCase

Comments

  • Single line: # Comment (capitalized, on own line)
  • Multi-line: <# Comment #> format (opening and closing brackets on own line), and indent text
  • No commented-out code

Comment-based help

  • Always add comment-based help to all functions and scripts
  • Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
  • Comment-based help indentation: keywords 4 spaces, text 8 spaces
  • Include examples for all parameter sets and combinations
  • INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...

Files:

  • source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1
source/DSCResources/**/*.psm1

⚙️ CodeRabbit configuration file

source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources Guidelines

Required Functions

  • Every DSC resource must define: Get-TargetResource, Set-TargetResource, Test-TargetResource
  • Export using *-TargetResource pattern

Function Return Types

  • Get-TargetResource: Must return hashtable with all resource properties
  • Test-TargetResource: Must return boolean ($true/$false)
  • Set-TargetResource: Must not return anything (void)

Parameter Guidelines

  • Get-TargetResource: Only include parameters needed to retrieve actual current state values
  • Get-TargetResource: Remove non-mandatory parameters that are never used
  • Set-TargetResource and Test-TargetResource: Must have identical parameters
  • Set-TargetResource and Test-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help comment

Required Elements

  • Each function must include Write-Verbose at least once
    • Get-TargetResource: Use verbose message starting with "Getting the current state of..."
    • Set-TargetResource: Use verbose message starting with "Setting the desired state of..."
    • Test-TargetResource: Use verbose message starting with "Determining the current state of..."
  • Use localized strings for all messages (Write-Verbose, Write-Error, etc.)
  • Import localized strings using Get-LocalizedData at module top

Error Handling

Files:

  • source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1
**/*.md

⚙️ CodeRabbit configuration file

**/*.md: # Markdown Style Guidelines

  • Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
  • Use 2 spaces for indentation
  • Use '1.' for all items in ordered lists (1/1/1 numbering style)
  • Disable MD013 rule by adding a comment for tables/code blocks exceeding 80 characters
  • Empty lines required before/after code blocks and headings (except before line 1)
  • Escape backslashes in file paths only (not in code blocks)
  • Code blocks must specify language identifiers

Text Formatting

  • Parameters: bold
  • Values/literals: inline code
  • Resource/module/product names: italic
  • Commands/files/paths: inline code

Files:

  • CHANGELOG.md
CHANGELOG.md

⚙️ CodeRabbit configuration file

CHANGELOG.md: # Changelog Guidelines

  • Always update the Unreleased section in CHANGELOG.md
  • Use Keep a Changelog format
  • Describe notable changes briefly, ≤2 items per change type
  • Reference issues using format issue #<issue_number>
  • No empty lines between list items in same section
  • Skip adding entry if same change already exists in Unreleased section
  • No duplicate sections or items in Unreleased section

Files:

  • CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: dsccommunity.CommonTasks (Build Package Module)
🔇 Additional comments (2)
source/DSCResources/OfficeOnlineServerSetup/OfficeOnlineServerSetup.schema.psm1 (1)

98-102: DependsOn addition is correct.

Including NetFx35 in the service dependency is appropriate; extend it if you split 3.5 sub‑features per above.

tests/Unit/DSCResources/Assets/Config/OfficeOnlineServerSetup.yml (1)

7-7: LGTM — WindowsFeatureSourcePath present; verify other usages.

tests/Unit/DSCResources/Assets/Config/OfficeOnlineServerSetup.yml:7 — WindowsFeatureSourcePath: \server\Data\sxs. The verification scan for PowerShell usages did not complete (rg file-type filter + awk syntax error); confirm all OfficeOnlineServerSetup invocations pass this now-mandatory parameter.

@raandree raandree merged commit 4376ba7 into dsccommunity:main Sep 22, 2025
6 checks passed
@raandree raandree deleted the fix/dotnet3Source branch September 22, 2025 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant