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

How to express arguments? #171

Open
grokys opened this issue Nov 15, 2018 · 1 comment
Open

How to express arguments? #171

grokys opened this issue Nov 15, 2018 · 1 comment
Labels
Status: Needs info Full requirements are not yet known, so implementation should not be started Type: Feature New feature or request

Comments

@grokys
Copy link
Collaborator

grokys commented Nov 15, 2018

We're currently passing arguments using standard C# parameters, but this has a couple of problems:

  1. Since Sort method arguments. #166, the arguments are now sorted by name. This means if an argument is added to the schema, it could show up in the middle of existing arguments breaking our API.
  2. In theory we can use named arguments (which would also solve 1) but in reality we often can't because C# expressions don't support named arguments, leading to some horrible null, null, null, null business. And because of this, adding an argument to the schema is a breaking change.

@StanleyGoldman suggested doing something like this instead:

Repository(x => x.Owner("Owner").Name("Name"))

Pros:

  1. Additions to schema no longer break our API
  2. Will no longer see null, null, null, null in expressions

Cons:

  1. Will need to generate a lot more code
  2. Moves us further away from looking like "just C#" code

I was originally against this idea because of the cons above, but I'm starting to think the pros outweigh the cons, especially given that it looks like C#'s expression support isn't going to be improved anytime soon (despite some proposals).

@github-actions
Copy link

github-actions bot commented Dec 4, 2022

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Dec 4, 2022
@kfcampbell kfcampbell added Priority: Normal Type: Feature New feature or request Status: Needs info Full requirements are not yet known, so implementation should not be started labels Dec 5, 2022
@github-actions github-actions bot removed the Status: Stale Used by stalebot to clean house label Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs info Full requirements are not yet known, so implementation should not be started Type: Feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants