Skip to content

Conversation

@josesimoes
Copy link
Member

@josesimoes josesimoes commented Apr 29, 2025

Description

  • Merge with main.

Motivation and Context

  • Used for unit tests in interpreter.

How Has This Been Tested?

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
  • I have added new tests to cover my changes.

Summary by CodeRabbit

  • Chores

    • Simplified pipeline triggers and adjusted release gating to consider main and develop branches.
    • Switched to semantic preview versioning (preview height-based build number) and added an extra branch pattern for public releases.
    • Added options to pipeline templates to support preview builds and set the base branch for publishing.
  • New Features

    • Added a pipeline check that detects referenced nanoCLR preview builds in PRs and can download and expose the preview runtime for testing.

- Publish to nuget now happens on build from develop branch too.
- Temporary disable trigger for tags.

***NO_CI***
- Fix parameter name.
- Add branch to publish nugets from.

***NO_CI***
@coderabbitai
Copy link

coderabbitai bot commented Apr 29, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

CI pipeline and versioning were updated: azure-pipelines.yml changes template parameters, variables, triggers, and release conditions; a new template was added to extract nanoCLR build IDs from PRs and optionally download artifacts; version.json switched to semantic preview versioning and added a branch pattern.

Changes

Cohort / File(s) Change Summary
Pipeline root
azure-pipelines.yml
Removed top-level tag-based trigger filter; added DOTNET_NOLOGO: true under build_mscorlib; inserted check-nf-interpreter-to-test.yml template; passed usePreviewBuild: true to class-lib-build-only.yml; passed baseBranchName: 'develop' to class-lib-publish.yml; broadened release conditions to consider develop (via Build.SourceBranchName) and adjusted isPreRelease logic; minor reordering/cleanup.
New pipeline template
azure-pipelines-templates/check-nf-interpreter-to-test.yml
New template that calls GitHub REST API (using GITHUB_TOKEN) to read PR body, validates PR number, extracts a nanoCLR buildId via regex, sets NFINTERPRETER_BUILDID pipeline variable, conditionally downloads a nanoCLR preview artifact when enabled, and exposes NF_MDP_NANOCLR_INSTANCE_PATH.
Versioning config
version.json
Switched base version to 2.0.0-preview.{height} (preview tag with height-based build number) and added branch pattern ^refs/heads/generics$ to branches considered for public releases.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer
    participant AP as AzurePipelines
    participant GH as GitHub
    participant Art as ArtifactStorage

    Dev->>AP: Push / open PR
    AP->>AP: Start pipeline
    alt PR present
        AP->>AP: Run check-nf-interpreter-to-test template
        AP->>GH: GET /repos/.../pulls/{PR} (auth: GITHUB_TOKEN)
        GH-->>AP: PR payload (body)
        AP->>AP: Verify PR number, extract buildId via regex
        alt buildId found and DownloadNanoClrPreview = true
            AP->>Art: Download nanoCLR artifact (by buildId)
            Art-->>AP: artifact (nanoCLR dll)
            AP->>AP: Set NF_MDP_NANOCLR_INSTANCE_PATH variable
        else
            AP-->>AP: Skip download
        end
    end
    AP->>AP: Run class-lib-build-only (usePreviewBuild: true)
    AP->>AP: Run class-lib-publish (baseBranchName: develop)
    AP->>AP: Read version.json (2.0.0-preview.{height})
    AP->>AP: Evaluate release condition (branch main or develop, not StartReleaseCandidate)
    alt release conditions satisfied
        AP->>GH: Create GitHub release (isPreRelease when branch is develop)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title asserts that this PR merges generics support into mscorlib, but the changeset consists solely of CI pipeline configuration and versioning updates without any mscorlib code modifications related to generics. This discrepancy means the title does not accurately summarize the main content of the changes. Please revise the PR title to reflect the actual scope of the changes—such as updating Azure Pipelines and versioning for generics testing—or include the intended generics support code changes in mscorlib so that the title matches the diff.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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: 0

🧹 Nitpick comments (2)
azure-pipelines.yml (2)

59-60: Remove duplicate variable declaration & trailing spaces.

You’ve declared DOTNET_NOLOGO both globally (lines 36–37) and again under the Build_mscorlib job. This duplication is unnecessary and may lead to confusion. Also, YAMLlint flagged trailing spaces on line 60.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 60-60: trailing spaces

(trailing-spaces)


132-136: Consider adding the generics branch to release conditions.

You’ve broadened the GitHub release condition to run on main and develop. To stay consistent with the new publicReleaseRefSpec, you might also include generics here if you intend to create releases from that branch.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cca059e and e37e4b8.

⛔ Files ignored due to path filters (11)
  • .runsettings is excluded by none and included by none
  • Tests/NFUnitTestGC/TestGC.cs is excluded by none and included by none
  • Tests/NFUnitTestSystemLib/UnitTestGCTest.cs is excluded by none and included by none
  • Tests/NFUnitTestSystemLib/UnitTestInitLocalTests.cs is excluded by none and included by none
  • Tests/NFUnitTestSystemLib/UnitTestReflectionTypeTest.cs is excluded by none and included by none
  • Tests/UnitTestLauncher/UnitTestLauncher.nfproj is excluded by none and included by none
  • nanoFramework.CoreLibrary.nuspec is excluded by none and included by none
  • nanoFramework.CoreLibrary/System/AssemblyInfo.cs is excluded by none and included by none
  • nanoFramework.CoreLibrary/System/GC.cs is excluded by none and included by none
  • nanoFramework.CoreLibrary/System/Guid.cs is excluded by none and included by none
  • nanoFramework.TestFramework is excluded by none and included by none
📒 Files selected for processing (2)
  • azure-pipelines.yml (3 hunks)
  • version.json (2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
azure-pipelines.yml

[error] 60-60: trailing spaces

(trailing-spaces)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: nanoframework.CoreLibrary (Build_mscorlib)
  • GitHub Check: nanoframework.CoreLibrary (Build_mscorlib)
🔇 Additional comments (4)
version.json (2)

3-3: Approve semantic version update.

Updating the base version to "2.0.0-preview.{height}" correctly switches to semantic versioning with a preview suffix. This aligns with the pipeline change enabling preview builds.


14-14: Approve new public release branch pattern.

Adding ^refs/heads/generics$ to publicReleaseRefSpec ensures the newly merged “generics” branch is considered for public releases.

azure-pipelines.yml (2)

79-79: Verify template supports usePreviewBuild.

You’ve enabled usePreviewBuild: true in the class-lib-build-only.yml invocation. Please confirm that the template accepts and correctly propagates this parameter to invoke preview-mode builds as expected.


122-125: Ensure publish step aligns with new branches.

The class-lib-publish.yml template is now called with baseBranchName: 'develop'. Given that version.json includes a generics branch for public releases, please verify whether baseBranchName should dynamically reflect generics (or accept a list of branches) to fully support that branch.

@sonarqubecloud
Copy link

- Add target for .NET Framework v4.7.2
@sonarqubecloud
Copy link

dcyonce and others added 5 commits November 24, 2025 12:38
(cherry picked from commit 81808e5)
- Remove unexistent var from conditions.

***NO_CI***
- Improve download and usage of specific nanoCLR from nf-interpreter build.

***NO_CI***
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 6, 2025

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

Labels

Status: in progress ⚠️ DO NOT MERGE ⚠️ This is not to be merged. In doubt check with the person that put the label.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants