Transition to multi-package repository #125
Conversation
nfbot
changed the title
WalkthroughThe pull request updates several CI/CD and dependency management configurations. The changelog template now lists two NuGet packages instead of one. Two GitHub Actions workflow files have been modified to add and properly format a solution parameter. The Azure Pipelines configuration has been restructured with new jobs and conditional triggers based on version tags. Additionally, new Changes
Sequence Diagram(s)sequenceDiagram
participant SCM as Source Control
participant AP as Azure Pipelines
participant BL as Build_Library
participant UD as Update_Dependents
participant RB as Report_Build_Failure
SCM->>AP: Push commit or tag
AP->>BL: Trigger Build_Library (if branch is not a version tag)
BL-->>AP: Build result (success/failure)
AP->>UD: Trigger Update_Dependents (based on tag/commit conditions)
UD-->>AP: Update result (success/failure)
AP->>RB: Evaluate job outcomes
RB-->>AP: Report failures via Discord webhook (if any)
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
azure-pipelines.yml (4)
36-39: Variables block formatting improvement.
There are some trailing spaces within the variables block (e.g., after line 38). Cleaning these up will improve readability and help avoid potential YAML lint warnings.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 39-39: trailing spaces
(trailing-spaces)
43-92: Build_Library job configuration review.
TheBuild_Libraryjob is structured well with clear conditions and step templates for building, packaging, and publishing the library. One suggestion is to address several trailing whitespace and indentation issues (for instance, around lines 67, 74, 78, 82, and 86) as noted by YAMLlint. This cleanup will enhance maintainability and prevent linting errors.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 43-43: wrong indentation: expected 2 but found 0
(indentation)
[error] 58-58: trailing spaces
(trailing-spaces)
[warning] 67-67: wrong indentation: expected 4 but found 2
(indentation)
[error] 74-74: trailing spaces
(trailing-spaces)
[error] 78-78: trailing spaces
(trailing-spaces)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 86-86: trailing spaces
(trailing-spaces)
118-130: Update_Dependents job condition clarity.
The combined conditions (checking tag prefixes, commit messages, and theUPDATE_DEPENDENTSflag) are logically sound and accommodate multiple update scenarios. As a minor note, please review the indentation and remove any trailing spaces to adhere to YAML best practices.
145-170: Report_Build_Failure job configuration.
This job effectively aggregates failures from theBuild_LibraryandUpdate_Dependentsjobs and reports them via Discord. To ensure a clean and error-free YAML file, please address the trailing spacing and indentation issues (notably on lines 146, 148, 160, 161, and 165) as highlighted by the static analysis.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 146-146: trailing spaces
(trailing-spaces)
[warning] 148-148: wrong indentation: expected 4 but found 2
(indentation)
[error] 160-160: trailing spaces
(trailing-spaces)
[warning] 161-161: wrong indentation: expected 4 but found 2
(indentation)
[error] 165-165: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (45)
.editorconfigis excluded by none and included by noneREADME.mdis excluded by!**/*.mdand included by noneStub-generation.mdis excluded by!**/*.mdand included by nonenanoFramework.Device.Can.Core.nuspecis excluded by none and included by nonenanoFramework.Device.Can.Core/CanController.csis excluded by none and included by nonenanoFramework.Device.Can.Core/CanControllerEventListener.csis excluded by none and included by nonenanoFramework.Device.Can.Core/CanControllerManager.csis excluded by none and included by nonenanoFramework.Device.Can.Core/CanEvent.csis excluded by none and included by nonenanoFramework.Device.Can.Core/CanMessage.csis excluded by none and included by nonenanoFramework.Device.Can.Core/CanMessageEvent.csis excluded by none and included by nonenanoFramework.Device.Can.Core/CanMessageFrameType.csis excluded by none and included by nonenanoFramework.Device.Can.Core/CanMessageIdType.csis excluded by none and included by nonenanoFramework.Device.Can.Core/CanMessageReceivedEventArgs.csis excluded by none and included by nonenanoFramework.Device.Can.Core/CanSettings.csis excluded by none and included by nonenanoFramework.Device.Can.Core/Properties/AssemblyInfo.csis excluded by none and included by nonenanoFramework.Device.Can.Core/key.snkis excluded by none and included by nonenanoFramework.Device.Can.Core/nanoFramework.Device.Can.Core.nfprojis excluded by none and included by nonenanoFramework.Device.Can.Core/packages.configis excluded by none and included by nonenanoFramework.Device.Can.Esp32.nuspecis excluded by none and included by nonenanoFramework.Device.Can.Esp32/Properties/AssemblyInfo.csis excluded by none and included by nonenanoFramework.Device.Can.Esp32/key.snkis excluded by none and included by nonenanoFramework.Device.Can.Esp32/nanoFramework.Device.Can.Esp32.nfprojis excluded by none and included by nonenanoFramework.Device.Can.Esp32/packages.configis excluded by none and included by nonenanoFramework.Device.Can.Mcp2515.nuspecis excluded by none and included by nonenanoFramework.Device.Can.Mcp2515/Properties/AssemblyInfo.csis excluded by none and included by nonenanoFramework.Device.Can.Mcp2515/key.snkis excluded by none and included by nonenanoFramework.Device.Can.Mcp2515/nanoFramework.Device.Can.Mcp2515.nfprojis excluded by none and included by nonenanoFramework.Device.Can.Mcp2515/packages.configis excluded by none and included by nonenanoFramework.Device.Can.Stm32.nuspecis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanController.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanControllerEventListener.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanControllerManager.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanEvent.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanMessage.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanMessageEvent.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanMessageFrameType.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanMessageIdType.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanMessageReceivedEventArgs.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/CanSettings.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/Properties/AssemblyInfo.csis excluded by none and included by nonenanoFramework.Device.Can.Stm32/key.snkis excluded by none and included by nonenanoFramework.Device.Can.Stm32/nanoFramework.Device.Can.Stm32.nfprojis excluded by none and included by nonenanoFramework.Device.Can.Stm32/packages.configis excluded by none and included by nonenanoFramework.Device.Can.slnis excluded by none and included by nonespelling_exclusion.dicis excluded by none and included by none
📒 Files selected for processing (7)
.github/.changelog-config.json(1 hunks).github/workflows/pr-checks.yml(1 hunks).github/workflows/update-dependencies.yml(2 hunks)azure-pipelines.yml(2 hunks)nanoFramework.Device.Can.Esp32/packages.lock.json(1 hunks)nanoFramework.Device.Can.Mcp2515/packages.lock.json(1 hunks)nanoFramework.Device.Can.Stm32/packages.lock.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- .github/workflows/update-dependencies.yml
- nanoFramework.Device.Can.Stm32/packages.lock.json
- nanoFramework.Device.Can.Mcp2515/packages.lock.json
- nanoFramework.Device.Can.Esp32/packages.lock.json
🧰 Additional context used
🪛 YAMLlint (1.35.1)
azure-pipelines.yml
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 39-39: trailing spaces
(trailing-spaces)
[warning] 43-43: wrong indentation: expected 2 but found 0
(indentation)
[error] 58-58: trailing spaces
(trailing-spaces)
[warning] 67-67: wrong indentation: expected 4 but found 2
(indentation)
[error] 74-74: trailing spaces
(trailing-spaces)
[error] 78-78: trailing spaces
(trailing-spaces)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 86-86: trailing spaces
(trailing-spaces)
[error] 139-139: trailing spaces
(trailing-spaces)
[warning] 140-140: wrong indentation: expected 4 but found 2
(indentation)
[error] 146-146: trailing spaces
(trailing-spaces)
[warning] 148-148: wrong indentation: expected 4 but found 2
(indentation)
[error] 160-160: trailing spaces
(trailing-spaces)
[warning] 161-161: wrong indentation: expected 4 but found 2
(indentation)
[error] 165-165: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: nanoFramework.Device.Can (Build_Library)
🔇 Additional comments (2)
.github/workflows/pr-checks.yml (1)
13-14: Addition of 'solution' parameter in the check_package_lock job.
The newwith: solution: 'nanoFramework.Device.Can.sln'parameter brings consistency with thecheck_nuget_latestjob and helps ensure the correct solution is targeted during package lock verification.azure-pipelines.yml (1)
20-23: Introduced tag-based triggers for builds.
Adding the tag trigger (include: - v*) ensures that builds can be automatically initiated for version tags, which is a key part of the release process for multiple packages.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 21-21: trailing spaces
(trailing-spaces)
| @@ -37,7 +37,7 @@ | |||
| } | |||
| ], | |||
| "sort": "ASC", | |||
| "template": "${{CHANGELOG}}\n\n**Full Changelog:** ${{RELEASE_DIFF}}\n\nThe following NuGet package is available from this release:\n\n:package: [nanoFramework.Devices.Can](https://www.nuget.org/packages/nanoFramework.Devices.Can/)", | |||
| "template": "${{CHANGELOG}}\n\n**Full Changelog:** ${{RELEASE_DIFF}}\n\nThe following NuGet packages are available from this release:\n\n:package: [nanoFramework.Devices.Can.Core](https://www.nuget.org/packages/nanoFramework.Devices.Can.Core/)\n:package: [nanoFramework.Device.Can.Stm32](https://www.nuget.org/packages/nanoFramework.Devices.Can.Stm32/)", | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Updated changelog template with multi-package information.
The revised template now details both NuGet packages—:package: [nanoFramework.Devices.Can.Core](...) and :package: [nanoFramework.Device.Can.Stm32](...)—which aligns with the multi-package repository strategy. However, please verify that the Core package’s name is consistent across the repository; the template uses "Devices" (plural) while other references use "Device" (singular).
Signed-off-by: Frank Robijn <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
azure-pipelines.yml (9)
20-23: Tags Block Addition and FormattingYou've added a new tags section to trigger builds on version tags (
v*), which aligns with the multi-package repository strategy. However, static analysis detected trailing spaces on line 21. Please remove the extra whitespace to adhere to YAML formatting standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 21-21: trailing spaces
(trailing-spaces)
55-62: Build_Library Job Variables DefinitionThe addition of variables (including the credentials group,
buildPlatform,buildConfiguration, andsolution) is well executed. However, static analysis indicates trailing spaces on line 58. Please remove the extra spaces to maintain a clean YAML file.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 58-58: trailing spaces
(trailing-spaces)
66-72: Build Step Template InvocationThe step that invokes the
class-lib-build-only.ymltemplate is structured correctly. Yet, static analysis warns that line 67 shows incorrect indentation (expected 4 spaces but found 2). Adjusting the indentation will help ensure the YAML parser processes this correctly.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 67-67: wrong indentation: expected 4 but found 2
(indentation)
73-77: Packaging Step for Core LibraryThe packaging step for
nanoFramework.Device.Can.Coreusing the designated template is correctly set up. Static analysis flags trailing spaces on line 74; please remove these spaces to conform with YAML lint standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 74-74: trailing spaces
(trailing-spaces)
78-85: Commented-Out Packaging StepsThe blocks for packaging
nanoFramework.Device.Can.Esp32andnanoFramework.Device.Can.Mcp2515are appropriately commented out. However, trailing spaces are reported on line 78 and line 82. Even in commented sections, it's beneficial to clean up formatting for readability.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 78-78: trailing spaces
(trailing-spaces)
[error] 82-82: trailing spaces
(trailing-spaces)
86-89: Packaging Step for Stm32 LibraryThe packaging step for
nanoFramework.Device.Can.Stm32looks correct. Please address the trailing spaces issue reported on line 86 to ensure a tidy configuration file.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 86-86: trailing spaces
(trailing-spaces)
138-142: Update_Dependents Job StepsIncluding the checkout step to persist GitHub credentials is essential. However, static analysis flags trailing spaces on line 139 and an indentation issue on line 140 (expected 4 spaces but found 2). Please correct these formatting issues to ensure consistency and proper parsing.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 139-139: trailing spaces
(trailing-spaces)
[warning] 140-140: wrong indentation: expected 4 but found 2
(indentation)
145-155: Report_Build_Failure Job DefinitionThe
Report_Build_Failurejob is set up to run when eitherBuild_LibraryorUpdate_Dependentsfails, which is a solid design for error reporting. Static analysis, however, reports trailing spaces on line 146 and a wrong indentation for thedependsOnentry on line 148. Addressing these formatting issues will improve the clarity and reliability of your YAML configuration.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 146-146: trailing spaces
(trailing-spaces)
[warning] 148-148: wrong indentation: expected 4 but found 2
(indentation)
159-170: Report_Build_Failure Job StepsThe steps for reporting build failures via a Discord webhook are well implemented. Attention should be paid to formatting details: static analysis flags trailing spaces on line 160, an indentation warning on line 161 (expected 4 spaces but found 2), and trailing spaces on line 165. Cleaning up these issues will help ensure the YAML file remains maintainable and error-free.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 160-160: trailing spaces
(trailing-spaces)
[warning] 161-161: wrong indentation: expected 4 but found 2
(indentation)
[error] 165-165: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
nanoFramework.Device.Can.Core/nanoFramework.Device.Can.Core.nfprojis excluded by none and included by nonenanoFramework.Device.Can.Esp32.nuspecis excluded by none and included by none
📒 Files selected for processing (1)
azure-pipelines.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
azure-pipelines.yml
[error] 21-21: trailing spaces
(trailing-spaces)
[warning] 43-43: wrong indentation: expected 2 but found 0
(indentation)
[error] 58-58: trailing spaces
(trailing-spaces)
[warning] 67-67: wrong indentation: expected 4 but found 2
(indentation)
[error] 74-74: trailing spaces
(trailing-spaces)
[error] 78-78: trailing spaces
(trailing-spaces)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 86-86: trailing spaces
(trailing-spaces)
[error] 139-139: trailing spaces
(trailing-spaces)
[warning] 140-140: wrong indentation: expected 4 but found 2
(indentation)
[error] 146-146: trailing spaces
(trailing-spaces)
[warning] 148-148: wrong indentation: expected 4 but found 2
(indentation)
[error] 160-160: trailing spaces
(trailing-spaces)
[warning] 161-161: wrong indentation: expected 4 but found 2
(indentation)
[error] 165-165: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: nanoFramework.Device.Can (Build_Library)
🔇 Additional comments (5)
azure-pipelines.yml (5)
38-39: DOTNET_NOLOGO Variable ConfigurationThe explicit setting of
DOTNET_NOLOGOtotrueis clear and correctly defined. No further changes are needed here, but do ensure consistency in variable usage across the pipeline.
40-42: Jobs Section DeclarationIntroducing the
jobs:section establishes the new structure required for a multi-package build system. The inclusion of this section looks good; just ensure that it harmonizes with the overall repository transition.
52-54: Build_Library Job Pool ConfigurationAssigning
vmImage: 'windows-latest'under the pool section is appropriate for the build environment. This section is clear and requires no changes.
118-130: Update_Dependents Job ConditionThe condition for the
Update_Dependentsjob—checking for version-tags, specific commit messages, or an explicit flag—is well structured and should support your dependency update strategy. Verify that these conditions align with your intended workflow triggers.
132-137: Update_Dependents Job Pool and VariablesThe pool setting and the definition of
DOTNET_NOLOGOfor theUpdate_Dependentsjob are clear and correctly specified. No issues detected here.
| - job: Build_Library | ||
| condition: >- | ||
| and( | ||
| not(startsWith(variables['Build.SourceBranch'], 'refs/tags/v')), | ||
| or( | ||
| eq(variables['UPDATE_DEPENDENTS'], 'false'), | ||
| eq(variables['StartReleaseCandidate'], 'true') | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Build_Library Job Condition
The condition for the Build_Library job correctly avoids triggering on version tag builds and handles the UPDATE_DEPENDENTS and StartReleaseCandidate flags. Note that static analysis highlights a potential indentation issue on line 43 (expected 2 spaces but found 0). Please verify and correct the YAML indentation if necessary to avoid parser warnings.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 43-43: wrong indentation: expected 2 but found 0
(indentation)
|
The build error seems to be caused by a mismatch between the dependencies in the nfproj and the ones in nuspec - not sure about that as I have no clue what the scripts are doing. If that is correct, the second commit should have fixed that, but it seems the build was not restarted. I guess we need some help to get this working. |
look at the differences of versions in vs proj file the |
|
I think the last commit should have fixed the version issues, but did not trigger the correct tasks. I'l re-create the PR. |
nfbot
added
invalid
and removed
Area: Config-and-Build
Type: documentation
⚠️ DO NOT MERGE ⚠️
labels
Description
Do not merge this PR!
It is the first step towards a multi-package repository. The PR is to make sure we're on the right track.
Transition of this Github repository into a repository for multiple NuGet packages/class libraries that collectively are the nanoFramework.Device.Can implementation in the near future.
Most work is done to achieve the correct build infrastructure:
The library code in this PR is not yet up to snuff:
The nanoFramework.Device.Can.Core project contains a complete copy of the current
nanoFramework.Device.Canlibrary. TheAssemblyNativeVersionattribute is removed for normal builds. In a subsequent PR the Stm32-specific code will be removed and optionally replaced by abstract base classes or interfaces.The nanoFramework.Device.Can.Stm32 project contains a complete copy of the current
nanoFramework.Device.Canlibrary. The version inAssemblyNativeVersionis changed to an arbitrary value. In a subsequent PR only the Stm32-specific code will be retained as an implementation of the classes and interfaces from the nanoFramework.Device.Can.Core library.The nanoFramework.Device.Can.Esp32 and nanoFramework.Device.Can.Mcp2515 projects are empty except for the assembly properties.
The subsequent PR that contains the correct versions of nanoFramework.Device.Can.Core and nanoFramework.Device.Can.Stm32 will also require a PR for the
nf-interpreterrepository as the stubs for the native components change. After the PR for both repositories are accepted and merged, the current nanoFramework.Device.Can NuGet package can be marked as deprecated.Motivation and Context
The conclusion of the discussion on the related PR.
How Has This Been Tested?
I have no idea how to test the Azure pipeline.
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Chores