Add Span<T> and ReadOnlySpan<T>
#249
Conversation
***WIP***
- Add missing null check in constructor. - Fix IntelliSense comments.
- Now calling native method (required to properly handle array copy operation). - Remove start field (not required).
- Remove start field. - Constructor with start param now calling native handler.
WalkthroughThe pipeline configuration removes tag triggers, adds DOTNET_NOLOGO variables, enables preview build in a build template, passes baseBranchName: develop to a publish template, and updates GitHub release conditions to only allow releases on main or develop when StartReleaseCandidate is false. version.json updates version to 2.0.0-preview.{height} and adds a new release branch. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Az as Azure Pipelines
participant B as Build_mscorlib Job
participant T1 as Template: class-lib-build-only
participant T2 as Template: class-lib-publish
participant GH as GitHub Release
Dev->>Az: Push/PR on branches (no tag triggers)
Az->>B: Queue pipeline (DOTNET_NOLOGO=true)
B->>T1: Invoke build (usePreviewBuild: true)
T1-->>B: Build artifacts
B->>T2: Invoke publish (baseBranchName: 'develop')
alt StartReleaseCandidate == false AND branchName in {main, develop}
Az->>GH: GithubRelease@1 create/update release
else other branches or StartReleaseCandidate == true
Az--xGH: Skip release
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
version.json (2)
3-3: Avoid hard-coding "-preview" in the version; let NBGV manage prerelease.Hard-coding "2.0.0-preview.{height}" can conflict with the release pipeline (e.g., GitHubRelease isPreRelease=false), producing a "stable" release whose tag still includes "-preview". Prefer a clean "2.0.0" and rely on firstUnstableTag: "preview" + publicReleaseRefSpec to add/remove the suffix automatically.
Suggested change:
- "version": "2.0.0-preview.{height}", + "version": "2.0.0",If keeping previews-only is intentional, flip the GitHubRelease isPreRelease flag accordingly (see pipeline comment).
14-15: Reconsider adding 'generics' as a public release branch.Including "^refs/heads/generics$" in publicReleaseRefSpec will mark builds on that branch as PublicRelease. This can yield stable SemVer (no prerelease tag) once version is not hard-coded to preview, and may contradict the pipeline’s release gating (main/develop only).
Options:
- Remove 'generics' from publicReleaseRefSpec if it’s a feature branch.
- Or, extend the pipeline’s release condition to also allow 'generics' to keep behavior aligned.
azure-pipelines.yml (2)
59-60: Trailing whitespace and duplicate variable.Line 60 has trailing spaces (yamllint error); also DOTNET_NOLOGO is already set globally at Line 36–37. Drop the duplicate here or keep only one definition.
Minimal fix:
- - name: DOTNET_NOLOGO - value: true + # DOTNET_NOLOGO is defined globally; remove local duplicateIf you prefer to keep it local, at least remove the trailing spaces:
- value: true + value: true
126-136: Align release comment and prerelease flag with actual behavior.
- The comment says “ON tags” but the condition now gates by branch (main/develop). Update the comment for accuracy.
- isPreRelease is hard-coded to false while version.json currently emits "-preview". This creates a stable GH release with a preview version tag. Drive isPreRelease from NBGV or branch.
Suggested tweaks:
- # create or update GitHub release ON tags + # create or update GitHub release on main/develop branches - isPreRelease: false + # mark prerelease when not a public release (from NBGV) + isPreRelease: $[ne(variables['NBGV_IsPublicRelease'], 'true')]If you keep 'generics' as PublicRelease in version.json, consider extending the branch gate:
- eq(variables['Build.SourceBranchName'], 'develop') + eq(variables['Build.SourceBranchName'], 'develop'), + eq(variables['Build.SourceBranchName'], 'generics')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (27)
.runsettingsis excluded by none and included by noneTests/NFUnitTestGC/TestGC.csis excluded by none and included by noneTests/NFUnitTestSystemLib/NFUnitTestSystemLib.nfprojis excluded by none and included by noneTests/NFUnitTestSystemLib/UnitTestGCTest.csis excluded by none and included by noneTests/NFUnitTestSystemLib/UnitTestInitLocalTests.csis excluded by none and included by noneTests/NFUnitTestSystemLib/UnitTestNullable.csis excluded by none and included by noneTests/NFUnitTestSystemLib/UnitTestReflectionTypeTest.csis excluded by none and included by noneTests/NFUnitTestTypes/UnitTestsSpanByte.csis excluded by none and included by noneTests/UnitTestLauncher/UnitTestLauncher.nfprojis excluded by none and included by nonenanoFramework.CoreLibrary.nuspecis excluded by none and included by nonenanoFramework.CoreLibrary/CoreLibrary.nfprojis excluded by none and included by nonenanoFramework.CoreLibrary/System/Array.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/ArrayTypeMismatchException.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/AssemblyInfo.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Collections/Generic/Comparer.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Collections/Generic/ComparerHelpers.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Collections/Generic/IComparer.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Diagnostics/StackTraceHiddenAttribute.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/GC.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Guid.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Nullable.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/ReadOnlySpan.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Runtime/InteropServices/InAttribute .csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Span.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/SpanDebugView.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/String.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Type.csis 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.37.1)
azure-pipelines.yml
[error] 60-60: trailing spaces
(trailing-spaces)
⏰ 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: nanoframework.CoreLibrary (Build_mscorlib)
🔇 Additional comments (2)
azure-pipelines.yml (2)
79-79: ConfirmusePreviewBuildis declared in your pipeline template
No occurrences found in any.ymltemplates (includingazure-pipelines-templates/class-lib-build-only.yml, which wasn’t located); ensure the template path is correct and that it declares and uses theusePreviewBuildboolean input before relying on it.
122-125: Verify newbaseBranchNameparameter wiring in external publish template
This template is pulled viaazure-pipelines-templates/class-lib-publish.yml@templates. Confirm that the remoteclass-lib-publish.ymlin thetemplatesrepo declares aparameters.baseBranchName(string) and uses it for branch targeting, tagging, or release notes.
josesimoes
changed the title
and ReadOnlySpan<T>`Span<T> and ReadOnlySpan<T>
|
2 participants
Description
Span<T>andReadOnlySpan<T>classes.Empty<T>toArrayclass.StackTraceHiddenAttribute.InAttribute.ArrayTypeMismatchException.Arraymethods.IsGenericTypeDefinitionis not virtual anymore.Typetests.Nullableunit tests.Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist:
Summary by CodeRabbit