-
-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
.NET8 Windows for SIL.Windows.Forms #1358
base: master
Are you sure you want to change the base?
Conversation
LibPalaso Tests 46 files + 1 46 suites +1 10m 49s ⏱️ +17s For more details on these failures, see this check. Results for commit 93206a7. ± Comparison against base commit 46fa5f0. This pull request removes 6 and adds 1 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
Updated L10NSharp to 8, which is the version that adds support for .NET8 Windows. Removed some old app.config settings that were causing build warnings. Updated some of the old .NET menu items for the newer style MenuStrip/MenuItem style. Also removed the old CAS-style PermissionSet attributes. This has precedent in FLEx, I believe. Also removed ImageToolbox controls -- they appear to be unused within SIL, and the third-party dependency is Framework 4.6.1 only. The other ancient third-party dependency we have is Enchant, which does appear to be used. But I have marked those two direct references as Obsolete, so we should see build warnings for consumers.
….Windows.Forms like all the others, but it doesn't.
With .NET8, the LocalApplicationData property has a different value than with Framework, causing a test failure in this class. That test ended reading the temp file from a previous test. I refactored the file pathing and directory creation to be adapted from the SettingsProvider itself, which should make it more flexible.
93206a7
to
ba9a4c5
Compare
Palaso Tests 3 files ± 0 3 suites ±0 15m 48s ⏱️ +15s Results for commit f4259bc. ± Comparison against base commit c06c991. This pull request removes 9 and adds 17 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
…nstant string. This was necessary to be more explicit in the test what is actually being fixed, while avoiding .Codebase, which produces a warning in modern .NET.
Also marked its test class with the same. Also marked another test class Windows-only, though it doesn't have to be Windows-only (just how it was written). This attribute is only supported with .NET5 and later, so on Framework there will be no marking. These changes fix a large number of build warnings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one minor issue, otherwise it looks good
@hahn-kev I'm seeing the Versions between the server and my local are similar, but: I don't think that would matter? |
Updated L10NSharp to 8, which is the version that adds support for .NET8 Windows.
Removed some old app.config settings that were causing build warnings.
Updated some of the old .NET menu items for the newer style MenuStrip/MenuItem style.
Also removed the old CAS-style PermissionSet attributes. This has precedent in FLEx, I believe.
Also removed ImageToolbox controls -- they appear to be unused within SIL, and the third-party dependency is Framework 4.6.1 only.
The other ancient third-party dependency we have is Enchant, which does appear to be used. But I have marked those two direct references as Obsolete, so we should see build warnings for consumers.
This change is