Conversation
|
🧙 Sourcery has finished reviewing your pull request! Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new inbox-fill-16 icon to the OctIcon component library and introduces conditional versioning based on Visual Studio version to support different framework versions.
- Adds the
inbox-fill-16icon SVG symbol to the icon library - Implements conditional versioning for Visual Studio 17.0 (version 9.0.7) and VS 18.0 (version 10.0.0-rc.2.1.0)
- Configures conditional package references for BootstrapBlazor dependency based on Visual Studio version
Reviewed Changes
Copilot reviewed 1 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| octicon.svg | Adds new inbox-fill-16 icon symbol for filled inbox variant |
| BootstrapBlazor.OctIcon.csproj | Introduces VS version-based conditional versioning and package references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| <ItemGroup> | ||
| <ItemGroup Condition="'$(VisualStudioVersion)' == '17.0'"> | ||
| <PackageReference Include="BootstrapBlazor" Version="$(BBVersion)" /> |
There was a problem hiding this comment.
The BootstrapBlazor package reference for VS 17.0 uses $(BBVersion) variable which is not defined in this project file, while VS 18.0 uses a hardcoded version. This inconsistency could cause build issues. Based on other similar projects (e.g., BootstrapBlazor.Region uses '9.12.0', BootstrapBlazor.Html2Pdf uses '9.12.0'), consider either defining the BBVersion property or using a specific version like the other files in the codebase.
| <PackageReference Include="BootstrapBlazor" Version="$(BBVersion)" /> | |
| <PackageReference Include="BootstrapBlazor" Version="9.12.0" /> |
| <PropertyGroup Condition="'$(VisualStudioVersion)' == '18.0'"> | ||
| <Version>10.0.0-rc.2.1.0</Version> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
[nitpick] The conditional versioning approach based on VisualStudioVersion property is inconsistent with the BootstrapBlazor.Topology.csproj file in the same repository, which uses a single unconditional ItemGroup for the BootstrapBlazor package reference. Consider whether this conditional approach is necessary for this specific component or if it should follow the simpler pattern used in Topology.
Link issues
fixes #642
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Bump OctIcon component to version 9.0.7, update SVG assets, and address issue #642
Bug Fixes:
Enhancements: