Closed andrew-polk closed 7 months ago
github-actions[bot]
commented
7 months ago 0 tests ±0 0 :heavy_check_mark: ±0 0s :stopwatch: ±0s 0 suites ±0 0 :zzz: ±0 0 files ±0 0 :x: ±0
Results for commit 68804263. ± Comparison against base commit 5e2b24ba.
:recycle: This comment has been updated with latest results.
andrew-polk
commented
7 months ago src/L10NSharpTests/LocalizationManagerTests_NoManagersLoaded.cs line 94 at r1 (raw file):
Was this commented line left in for a reason?
Nope! Fixed.
tombogle
commented
7 months ago src/L10NSharpTests/LocalizationManagerTests_NoManagersLoaded.cs line 33 at r1 (raw file):
Actually, it's because that code path has the following: ``` //this happens in unit test environments or apps that //have imported a library that is L10N'ized, but the app //itself isn't initializing L10N yet. if (LoadedManagers.Count == 0) { if (PreviouslyLoadedManagers.Contains(appId)) { if (LocalizationManager.ThrowIfManagerDisposed) { throw new ObjectDisposedException( $"The application id '{appId}' refers to a LocalizationManagerInternal that has been disposed"); } return string.IsNullOrEmpty(englishText) ? id : englishText; } if (!string.IsNullOrEmpty(englishText) && langId == LocalizationManager.kDefaultLang) return englishText; return id; } ``` which I didn't want to attempt to mess with. The good thing is that code path doesn't set the mapping, so it doesn't actually break anything. I'm definitely open to suggestions on how to improve commenting around this.
It would probably be safe to check that flagg and throw that same InvalidOperationException if there are 0 loaded managers and 0 previously loaded managers. But it's probably okay to just leave it as is.
+semver: major
This change is