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

Abstraction namespace #528

Merged
merged 9 commits into from
Aug 8, 2023
Merged

Abstraction namespace #528

merged 9 commits into from
Aug 8, 2023

Conversation

halgari
Copy link
Collaborator

@halgari halgari commented Aug 8, 2023

Working on the auto-updater, I realized I wanted CLI verbs for the updater but didn't want them in the CLI project. After our conversations today about moving interfaces into a generalized abstraction project, I took the liberty to rough this out for the CLI.

This PR does the following:

  • Adds NexusMods.Abstractions
  • Moves the CLI interfaces into NexusMods.Abstractions.CLI
  • Reworks all existing verbs to reference these new interfaces (a bit of untangling here was needed as the interfaces can't reference CLI logic)
  • Moved one verb nexus-api-verify into the project it belongs in, the Nexus API
  • Deletes the Configurator class. This class was used to late-bind the Renderer and allow for CLI options to define the renderer. This was never a very clean implementation, so instead I opted for a trait in this case: the verbs implement IRenderingVerb and the CLI configurator modifies this renderer before calling the verb. I wish there was a better way to do this, but Rendering can change after the verb is constructed based on the options passed into the CLI parser. What we have now with this patch is clean enough.

The only library required by the Abstractions namespace (thus far) is the Microsoft.Extensions.DependencyInjection.Abstractions library, which itself is lightweight.

As time goes on we can move other verbs into their proper libraries, but this unblocks my work and introduces an abstraction library which I think is a positive change for the project.

@halgari halgari requested a review from a team August 8, 2023 02:56
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.

Few comments that need cleanup. The CI has highlighted them

src/NexusMods.Abstractions/CLI/Verb.cs Outdated Show resolved Hide resolved
src/NexusMods.Abstractions/CLI/OptionDefinition.cs Outdated Show resolved Hide resolved
src/NexusMods.Abstractions/CLI/RegisteredVerb.cs Outdated Show resolved Hide resolved
@halgari halgari merged commit b65d109 into main Aug 8, 2023
3 of 5 checks passed
@halgari halgari added this to the v0.2 milestone Aug 8, 2023
@halgari halgari added the Tech: code-architecture-design Involves design or redesign of the code architecture label Aug 8, 2023
@erri120 erri120 deleted the abstraction-namespace branch August 8, 2023 12:30
@Greg-Nexus Greg-Nexus assigned Greg-Nexus and halgari and unassigned Greg-Nexus Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tech: code-architecture-design Involves design or redesign of the code architecture
4 participants