Skip to content

Conversation

@jasonleenaylor
Copy link
Contributor

@jasonleenaylor jasonleenaylor commented Sep 22, 2025

This change is Reviewable

Copilot AI review requested due to automatic review settings September 22, 2025 22:45
Copy link

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 fixes an issue where dialect labels were not being properly merged when importing duplicate lexical entries from XML data. The change ensures that when duplicate entries are detected during import, their dialect labels are combined rather than lost.

  • Added logic to merge dialect labels from old entries into new entries during import
  • Created a test case to verify dialect label merging works correctly with XML import data

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/SIL.LCModel/Application/ApplicationServices/XmlImportData.cs Added MergeDialectLabels method and integrated it into the entry merging process
tests/SIL.LCModel.Tests/Application/ApplicationServices/XmlImportDataTests.cs Added test case to verify dialect label merging functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

…ortDataTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions
Copy link

LCM Tests

    16 files  ±0      16 suites  ±0   3m 0s ⏱️ ±0s
 2 847 tests +1   2 827 ✅ +1   20 💤 ±0  0 ❌ ±0 
11 336 runs  +4  11 168 ✅ +4  168 💤 ±0  0 ❌ ±0 

Results for commit 25fec71. ± Comparison against base commit 62825aa.

Copy link
Contributor

@KenZook KenZook left a comment

Choose a reason for hiding this comment

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

Looks OK

@jasonleenaylor jasonleenaylor merged commit 4d3816d into master Sep 23, 2025
4 of 5 checks passed
@jasonleenaylor jasonleenaylor deleted the bugfix/LT-21976 branch September 23, 2025 14:33
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