Skip to content

Conversation

@bgn42
Copy link
Collaborator

@bgn42 bgn42 commented Jul 28, 2025

…so one in a correct level

Fixes

Changes

  • make additional check loop in verifying a pack when adding

Checklist

  • 🤖 This change is covered by unit tests (if applicable).
  • 🤹 Manual testing has been performed (if necessary).
  • 🛡️ Security impacts have been considered (if relevant).
  • 📖 Documentation updates are complete (if required).
  • [xx] 🧠 Third-party dependencies and TPIP updated (if required).

@bgn42 bgn42 requested review from Copilot and jkrech July 28, 2025 15:33
@qltysh
Copy link

qltysh bot commented Jul 28, 2025

Diff Coverage: The code coverage on the diff in this pull request is 84.7%.

Total Coverage: This PR will increase coverage by 0.02%.

File Coverage Changes
Path File Coverage Δ Indirect
cmd/installer/pack.go -1.1
🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

This comment was marked as outdated.

jkrech
jkrech previously approved these changes Jul 29, 2025
Copy link
Member

@jkrech jkrech left a comment

Choose a reason for hiding this comment

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

LGTM

@bgn42 bgn42 requested a review from Copilot July 30, 2025 08:21
Copy link
Contributor

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 improves the validation logic for PDSC files when adding CMSIS packs. The change enhances the pack validation process to show warnings instead of errors when a pack contains a valid PDSC file at the correct level alongside additional PDSC files that are too deep in the folder structure.

  • Refactored the validation loop to collect all valid PDSC files before making decisions
  • Added logic to show warnings for additional deep PDSC files when valid ones exist at correct levels
  • Introduced new error types for multiple PDSC files and wrong PDSC names

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
cmd/installer/pack.go Restructured pack validation logic to handle PDSC files more intelligently with warning/error prioritization
cmd/errors/errors.go Added two new error types for multiple PDSC files and wrong PDSC file names
Comments suppressed due to low confidence (1)

cmd/installer/pack.go:277

  • [nitpick] The variable name file is too generic and shadows the loop variable from the earlier iteration. Consider renaming it to pdscFile or validPdscFile for clarity.
	file := validPdscFiles[0]

@bgn42 bgn42 requested a review from jkrech August 13, 2025 09:30
@bgn42 bgn42 merged commit 34473fb into main Aug 13, 2025
2 checks passed
@bgn42 bgn42 deleted the pdscDeepInPack branch August 13, 2025 10:24
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.

3 participants