-
Notifications
You must be signed in to change notification settings - Fork 43
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
….Abstractions. The Configurator class previously was used to inject renderers into verbs in a indirect way. Instead now we supply these renderers via a optional trait interface
erri120
requested changes
Aug 8, 2023
Al12rs
reviewed
Aug 8, 2023
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.
Few comments that need cleanup. The CI has highlighted them
erri120
approved these changes
Aug 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
NexusMods.Abstractions
NexusMods.Abstractions.CLI
nexus-api-verify
into the project it belongs in, the Nexus APIIRenderingVerb
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.