Skip to content

Changelog for the Keyman Legacy Bundle dependency#1436

Merged
josephmyers merged 1 commit intomasterfrom
document-keyman-legacy-nuget
May 26, 2025
Merged

Changelog for the Keyman Legacy Bundle dependency#1436
josephmyers merged 1 commit intomasterfrom
document-keyman-legacy-nuget

Conversation

@josephmyers
Copy link
Collaborator

@josephmyers josephmyers commented May 23, 2025

Closes #1434


This change is Reviewable

@josephmyers
Copy link
Collaborator Author

I put this in the released version, but I can moved it to Unreleased, if desired. Also, @andrew-polk mentioned that this change was breaking. I either missed or forgot that, and how it was. I am happy to document this, if someone can help me improve the wording.

@josephmyers josephmyers requested a review from andrew-polk May 23, 2025 04:33
@github-actions
Copy link

github-actions bot commented May 23, 2025

Palaso Tests

     4 files  ±0       4 suites  ±0   10m 16s ⏱️ -51s
 4 978 tests ±0   4 744 ✅ +1  234 💤  - 1  0 ❌ ±0 
16 186 runs  ±0  15 483 ✅ +2  703 💤  - 2  0 ❌ ±0 

Results for commit 2743a73. ± Comparison against base commit 3590b5a.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@tombogle tombogle left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @andrew-polk)


CHANGELOG.md line 77 at r1 (raw file):

- [SIL.Core] BREAKING CHANGE (potentially): Changed behavior of DateTimeExtensions.ParseDateTimePermissivelyWithException (now deprecated) to try to interpret the date according to either the Gregorian calendar or the Buddhist calender in order to get the date to fall within a reasonable expected range (from 1/1/1900 through one day in the future). This means that depending on the current culture, dates might be interpreted differently from before. The known places in SIL code where this method is used seems to be for dates in the recent past (modern times); hence the default range. A new overload was added that will allow callers with other needs to specify a different range.
- [SIL.Windows.Forms] BREAKING CHANGE: ImageToolbox Removed support for Linux due to no longer using DialogAdapters. Affects `OpenFileDialogWithViews`
- [SIL.Windows.Forms.Keyboarding] Moved Keyman legacy libraries to Nuget to fix runtime errors in modern .NET builds.

This should indicate that this is a breaking change since it requires package references to be updated. According to Semantic Versioning (SemVer), moving DLLs from one package to a separate package is considered a breaking change.
Package consumers may rely on transitive dependencies being directly referenced. When you move the DLL, consumers who had build-time or runtime expectations (e.g., direct assembly references, reflection loading, or copying to output) may now experience failures or missing references if they don’t update their dependencies explicitly.
Restore/build behavior can change. Some consumers may have constraints or tooling that doesn’t handle transitive dependency resolution well, especially in older environments (e.g., .NET Framework projects with packages.config or legacy MSBuild behavior).

@josephmyers josephmyers force-pushed the document-keyman-legacy-nuget branch from 85b0749 to 2743a73 Compare May 23, 2025 06:39
@josephmyers
Copy link
Collaborator Author

Changed. I'm sure you're right, but keep in mind that the DLL and its output location is the same. Can consumers reference its place in the repo? Not normal ones. Anyone depending on those DLL's, given how they were included, would have had to be referencing where all the libpalaso and related lib's were being resolved to, even for direct referencing, copying to output, etc., and this didn't change.

I guess unless they set up their project to be dependent on the libpalaso source itself, i.e. in a particular folder, and if they happen to be referencing the DLL's where they were checked in, then this would break. I just feel bad for them.

Copy link
Contributor

@tombogle tombogle left a comment

Choose a reason for hiding this comment

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

When I updated to the new version, it (automatically) added a reference to my packages.config file. It didn't require a change to any of my code or installer. But I guess the way SemVer defines it, that was a "breaking change." It's ever so slightly possibly that there are other subtle aspects to the change which could break something, but in this case, probably not.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @andrew-polk)

Copy link
Contributor

@andrew-polk andrew-polk left a comment

Choose a reason for hiding this comment

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

Sorry; it has been too long since I did this. I don't remember any details.
I think I remember it caused build failures. But maybe something just didn't update correctly the first time?
Thanks for adding this.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @josephmyers)

@josephmyers josephmyers merged commit 23dc405 into master May 26, 2025
12 checks passed
@josephmyers josephmyers deleted the document-keyman-legacy-nuget branch May 26, 2025 00:29
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.

KeymanLegacyBundle dependency not documented in CHANGELOG

3 participants