Skip to content

[ENG] Fix namespace collision between src and generated submodule #28196

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

Merged
merged 2 commits into from
Jul 16, 2025

Conversation

NoriZC
Copy link
Contributor

@NoriZC NoriZC commented Jul 16, 2025

Description

This pull request introduces changes to improve how namespaces are handled in the build scripts and templates for the Az modules. The most important updates include adding support for reading the RootNamespace from .csproj files, passing the namespace to template generation, and updating the template to include a placeholder for the namespace.

This could help resolve the issue in
#27943
#28101

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • Update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

@Copilot Copilot AI review requested due to automatic review settings July 16, 2025 11:13
Copy link

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances how module namespaces are handled during generation by reading the RootNamespace from existing .csproj files, passing it into the template engine, and updating the template/props files accordingly.

  • Added a <RootNamespace> placeholder to the project template.
  • Extended the PowerShell build script to extract and forward the namespace into template rendering.
  • Removed the hardcoded RootNamespace in Az.autorest.props to allow explicit template insertion.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tools/BuildScripts/Templates/Az.ModuleName.csproj Insert <RootNamespace> placeholder in the template
tools/BuildScripts/BuildScripts.psm1 Parse RootNamespace from .csproj and pass it to the template
src/Az.autorest.props Remove default <RootNamespace>, deferring to the new template
Comments suppressed due to low confidence (1)

tools/BuildScripts/BuildScripts.psm1:183

  • The variable subModuleNameTrimmed is undefined here. It looks like cSubModuleNameTrimmed was assigned above and should be used to construct the path.
    $csprojPath = Join-Path $SourceDirectory "Az.${subModuleNameTrimmed}.csproj"

Comment on lines +258 to +260
if ($Namespace) {
$templateFile = $templateFile -replace '{NamespacePlaceHolder}', $Namespace
}
Copy link
Preview

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

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

When $Namespace is empty or undefined, the placeholder {NamespacePlaceHolder} will remain in the generated .csproj, causing invalid XML. Consider providing a default or emitting an error if the namespace cannot be read.

Suggested change
if ($Namespace) {
$templateFile = $templateFile -replace '{NamespacePlaceHolder}', $Namespace
}
if (-not $Namespace) {
Write-Error "Error: Namespace is undefined or empty. Cannot generate valid .csproj file."
return
}
$templateFile = $templateFile -replace '{NamespacePlaceHolder}', $Namespace

Copilot uses AI. Check for mistakes.

@VeryEarly VeryEarly enabled auto-merge (squash) July 16, 2025 12:39
@VeryEarly VeryEarly merged commit cee760b into main Jul 16, 2025
12 checks passed
@VeryEarly VeryEarly deleted the nori/namespace branch July 17, 2025 01:48
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.

2 participants