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

Changed IConfigurator to return IConfigurator instead of void #1762

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

byte2pixel
Copy link

@byte2pixel byte2pixel commented Feb 19, 2025

Fixes #1219

  • I have read the Contribution Guidelines
  • I have commented on the issue above and discussed the intended changes
  • A maintainer has signed off on the changes and the issue was assigned to me
    • see above link to discussion.
  • All newly added code is adequately covered by tests
  • All existing tests are still running without errors
  • The documentation was modified to reflect the changes OR no documentation changes are required.

Changes

Changed IConfigurator to return IConfigurator instead of void for the following interface methods:

  • SetHelpProvider
  • SetHelpProvider<T>
  • AddExample

However, other methods like AddCommand narrow the configurator so the order of chaining methods does matter. Just to point that out. Chaining is optional obviously.

I saw the ICofiguratorOfT also returns void for SetDescription, and AddExample. However, it becomes a bigger challenge to change that because Configurator<T> implements both IUnsafeBranchConfigurator and IConfigurator<T> and both share the SetDescription and AddExample but one does not take in the generic type. Probably would need "unsafe configurator of T" or something?


Please upvote 👍 this pull request if you are interested in it.

- SetHelpProvider
- SetHelpProvider<T>
- AddExample

However, other methods like AddCommand narrow the configurator so order ends up making a difference.
@FrankRay78
Copy link
Contributor

Thanks, I'll review this shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change the IConfigurator Extension Methods to all return IConfigurator.
2 participants