Skip to content

Don't create watcher for "other" directory if there isn't one (BL-15838)#7660

Open
JohnThomson wants to merge 1 commit intoVersion6.3from
checkOtherExistsWatcher
Open

Don't create watcher for "other" directory if there isn't one (BL-15838)#7660
JohnThomson wants to merge 1 commit intoVersion6.3from
checkOtherExistsWatcher

Conversation

@JohnThomson
Copy link
Contributor

@JohnThomson JohnThomson commented Feb 5, 2026


Open with Devin

This change is Reviewable

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Choose a reason for hiding this comment

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

🚩 Pre-existing bug: Dictionary key access without existence check

The color palette merging code at lines 689-702 iterates over repoColorPalettes.Keys and directly accesses localColorPalettes[key] at line 693 without verifying the key exists in localColorPalettes. If the repo's colorPalettes.json contains a palette key that the local file doesn't have, this will throw a KeyNotFoundException.

The MergeColorPaletteValues function (FolderTeamCollection.cs:616-632) does handle null/empty values gracefully, but it never gets called if the dictionary access throws first.

This is pre-existing code, not introduced by this PR, but worth noting as it could cause sync failures when palette files diverge.

(Refers to lines 689-702)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

1 participant