Skip to content
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

Net 9 and Update Deps #2251

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Net 9 and Update Deps #2251

wants to merge 8 commits into from

Conversation

halgari
Copy link
Collaborator

@halgari halgari commented Nov 14, 2024

This PR does the following:

  • Moves the app over to .NET 9
  • Updates the action files to use the latest versions and .NET 9
  • Updates MnemonicDB and TransparentValueObjects
  • Fixes the code to match the new APIs in the above libraries

@halgari halgari marked this pull request as ready for review November 14, 2024 19:50
@@ -30,7 +30,7 @@ jobs:

build-and-test:
if: github.event_name == 'push' || github.event.pull_request.draft == false
uses: Nexus-Mods/NexusMods.App.Meta/.github/workflows/dotnet-build-and-test-with-osx.yaml@ae64a3be780a74e94b59ee463a413083013c8b0c
uses: Nexus-Mods/NexusMods.App.Meta/.github/workflows/dotnet-build-and-test-with-osx.yaml@main
Copy link
Contributor

Choose a reason for hiding this comment

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

Not using a pinned version of the workflows could cause issues.
Changes to the Meta repo that also require changes to the underlying repos would break CI during the mean time.
E.g. while the PR with the changes is being reviewed, Ci on all other PRs will break.

Just pointing it out, in case it others have opinions on this

extern/SMAPI Outdated Show resolved Hide resolved
Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

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

I added a couple of more db attributes in the LoadOrder v1 pr #2248
There are going to be conflicts there as I also created new modes that use our local attributes, those will need updating as well

@github-actions github-actions bot added the status-needs-rebase Set by CI do not remove label Nov 18, 2024
Copy link
Contributor

This PR conflicts with main. You need to rebase the PR before it can be merged.

@github-actions github-actions bot removed the status-needs-rebase Set by CI do not remove label Nov 18, 2024
Copy link
Contributor

This PR doesn't conflict with main anymore. It can be merged after all status checks have passed and it has been reviewed.

Copy link
Member

@erri120 erri120 left a comment

Choose a reason for hiding this comment

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

Some duplicate attributes still exist and should be removed. The new optional values should also be implemented instead of just defaulting to calling .Value where the status of the value is unclear. Use a try-get method or manual if-checks instead.

Copy link
Contributor

This PR conflicts with main. You need to rebase the PR before it can be merged.

@github-actions github-actions bot added the status-needs-rebase Set by CI do not remove label Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status-needs-rebase Set by CI do not remove
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants