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

[3/4] Improve: Small Cleanup / Tech Debt Reduction Around NexusWebApi / V2 Types. #2095

Merged
merged 23 commits into from
Oct 3, 2024

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented Sep 26, 2024

This is 3rd PR in chain. Continues from:

Should be merged after that PR, this will reduce the diff from main to only changed parts.

Next in chain (final PR):

Review final PR if you want to review all parts at once, else review them by parts sequentially.


Also contributes a bit towards:


Summary

This PR moves the types from

Into the mainline NexusWebApi abstractions project.
I refrained from doing that in #2092 because it would make PR reviewing very annoying.

In addition some work is done to help migration towards the V2 API.

Changelog

  • ModUpdates project now uses types in NexusWebApi abstractions.
  • Types which are confirmed to exist in V2 (GraphQL) API are placed in a V2 folder to prevent accidental future deletion when getting rid of REST APIs.
  • The structs ModId, FileId, GameId now use the correct field size (u32)
    • They are not u64 like in our previous definition. This has been verified with both backend and GraphQL documentation.
  • Implemented UidForFile and UidForMod.
    • These allow us to split up UID composite keys returned from the website into their specific components, (GameId+ ModId) or (GameId+FileId).
    • Methods to decode strings in V2 API results into Uid variants are provided.

@Sewer56 Sewer56 added the Epic: Tech-debt Technical debt, this needs solving in the long-term label Sep 26, 2024
@Sewer56 Sewer56 requested a review from a team September 26, 2024 06:18
@Sewer56 Sewer56 self-assigned this Sep 26, 2024
Copy link
Collaborator

@halgari halgari left a comment

Choose a reason for hiding this comment

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

Didn't mean to approve it above

@github-actions github-actions bot added the status-needs-rebase Set by CI do not remove label Sep 30, 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 Sep 30, 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.

@Sewer56 Sewer56 force-pushed the start-adding-v2-types branch from 22bc785 to 2c96668 Compare September 30, 2024 12:56
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 status-needs-rebase Set by CI do not remove and removed status-needs-rebase Set by CI do not remove labels Sep 30, 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.

@Sewer56 Sewer56 force-pushed the start-adding-v2-types branch from f8b190b to dbd6d20 Compare September 30, 2024 13:04
@github-actions github-actions bot added the status-needs-rebase Set by CI do not remove label Oct 1, 2024
Copy link
Contributor

github-actions bot commented Oct 1, 2024

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

Copy link
Contributor

github-actions bot commented Oct 2, 2024

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

@github-actions github-actions bot removed the status-needs-rebase Set by CI do not remove label Oct 2, 2024
@Sewer56 Sewer56 changed the title Improve: Small Cleanup / Tech Debt Reduction Around NexusWebApi / V2 Types. [3/4] Improve: Small Cleanup / Tech Debt Reduction Around NexusWebApi / V2 Types. Oct 2, 2024
@github-actions github-actions bot added the status-needs-rebase Set by CI do not remove label Oct 2, 2024
Copy link
Contributor

github-actions bot commented Oct 2, 2024

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

Copy link
Contributor

github-actions bot commented Oct 2, 2024

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

@github-actions github-actions bot removed the status-needs-rebase Set by CI do not remove label Oct 2, 2024
@halgari halgari dismissed their stale review October 3, 2024 15:50

changes were made as requested

# Conflicts:
#	src/Networking/NexusMods.Networking.ModUpdates/MultiFeedCacheUpdater.cs
#	src/Networking/NexusMods.Networking.ModUpdates/NexusMods.Networking.ModUpdates.csproj
#	src/Networking/NexusMods.Networking.ModUpdates/PerFeedCacheUpdater.cs
#	src/Networking/NexusMods.Networking.ModUpdates/PerFeedCacheUpdaterResult.cs
#	tests/Networking/NexusMods.Networking.ModUpdates.Tests/Helpers/TestItem.cs
#	tests/Networking/NexusMods.Networking.ModUpdates.Tests/MultiFeedCacheUpdaterTests.cs
#	tests/Networking/NexusMods.Networking.ModUpdates.Tests/PerFeedCacheUpdaterTests.cs
@github-actions github-actions bot added the status-needs-rebase Set by CI do not remove label Oct 3, 2024
Copy link
Contributor

github-actions bot commented Oct 3, 2024

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

Copy link
Contributor

github-actions bot commented Oct 3, 2024

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

@github-actions github-actions bot removed the status-needs-rebase Set by CI do not remove label Oct 3, 2024
@halgari halgari merged commit d09d737 into main Oct 3, 2024
12 checks passed
@halgari halgari deleted the start-adding-v2-types branch October 3, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic: Tech-debt Technical debt, this needs solving in the long-term
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants