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

feature(#2290): Use a param value as the default value of other param #2371

Merged
merged 4 commits into from
Nov 19, 2023

Conversation

jcagarcia
Copy link
Contributor

Hey! 👋

I think the feature requested in #2290 is pretty interesting. So I've added this PR for allowing to use a param value as the default value of some other parameter.

So now, we can do:

params do
  optional :color, type: String, default: 'blue'
  optional :primary_color, type: String, default: -> (params) { params[:color] }
end

I've updated the README for including this new feature. Don't know if the next version should be 2.1.0 instead of 2.0.1. Let me know if I should change it.

Thanks!!

@jcagarcia jcagarcia changed the title feature(#2290) Use a param value as the default value of other param feature(#2290): Use a param value as the default value of other param Nov 17, 2023
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Very nice! Yes, let's make this 2.1.0 in this PR. Thanks.

@jcagarcia
Copy link
Contributor Author

Thanks @dblock ! I've just updated the version to 2.1.0. Let me know if anything else is needed 👍

@dblock
Copy link
Member

dblock commented Nov 17, 2023

Stable release mentions 2.0.1 too, needs to be 2.1.0, thanks!

https://github.com/ruby-grape/grape#stable-release

@jcagarcia
Copy link
Contributor Author

Sure! I've just commit the change for updating the stable release section :) Hope it helps!

README.md Outdated
@@ -1242,6 +1242,17 @@ params do
end
```

You can use the value of one parameter as the default value of some other parameter. In this
case, if the `primary_color` parameter is not provided, it will have the same value as the `color`
one. If both of them not provided, both of them will have `blue` value.
Copy link
Member

Choose a reason for hiding this comment

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

One last thing, I promise that's it after that. We don't wrap text in markdown manually since it shows up, make this all 1-line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @dblock ! Don't worry at all, I really appreciate the reviews :)

I've just updated my addition to the README file for following 1-line format. However, I did it in that way because all the other sections of the document.

As you can see here:

https://github.com/ruby-grape/grape/blob/master/README.md?plain=1#L153-L159

If the 1-line is the new convention you want to follow, do you think I can help on moving all the README to 1-line format?

Copy link
Member

Choose a reason for hiding this comment

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

Absolutely. Maybe we can find a linter to run in PRs too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Let me try to PR this :)

@dblock dblock merged commit 34f90f8 into ruby-grape:master Nov 19, 2023
31 checks passed
@dblock
Copy link
Member

dblock commented Nov 19, 2023

Merged, thanks for hanging in here with me!

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.

2 participants