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

Default value lambda improvements #72

Merged
merged 4 commits into from May 29, 2020

Conversation

natesalisbury
Copy link
Contributor

Improvements to using a lambda as a default value:

  • Don't type_cast the actual lambda.
  • Type cast the result of calling a default lambda.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 08bd9ff on natesalisbury:default_value_lambdas into ** on aws:master**.

2 similar comments
@coveralls
Copy link

coveralls commented Jun 16, 2017

Coverage Status

Changes Unknown when pulling 08bd9ff on natesalisbury:default_value_lambdas into ** on aws:master**.

@coveralls
Copy link

coveralls commented Jun 16, 2017

Coverage Status

Changes Unknown when pulling 08bd9ff on natesalisbury:default_value_lambdas into ** on aws:master**.

@coveralls
Copy link

coveralls commented Jun 19, 2017

Coverage Status

Changes Unknown when pulling b5e0e8f on natesalisbury:default_value_lambdas into ** on aws:master**.

@natesalisbury natesalisbury changed the title Default value lambdas Default value lambda improvements Jun 19, 2017
@natesalisbury
Copy link
Contributor Author

@awood45 any feedback for these changes?

@awood45
Copy link
Member

awood45 commented Jun 23, 2017

Sorry, haven't had a chance to review yet. It's on my work backlog.

@natesalisbury
Copy link
Contributor Author

No worries, thanks!

@metaskills
Copy link

Was interested in using default values today and was surprised that this was not the behavior. In fact, would this PR solve a bug? I am not sure why storing a proc as a string would be the expected results?

@metaskills
Copy link

Like, is this the expected way to use a default value for a hash key?

class MyModel
  include Aws::Record
  string_attr       :prefix, hash_key: true
  string_attr       :description

  def initialize(*args)
    super
    self.prefix ||= new_prefix
  end

  # ...
end

@alextwoods
Copy link
Contributor

Apologies it has taken us so long to get around to reviewing this change. Overall it looks good, however, in ItemData#populate_default_values, the default_value lambda will end up getting called twice.

I'm happy to fix that later today.

@alextwoods alextwoods merged commit d9c6f1a into aws:master May 29, 2020
@natesalisbury natesalisbury deleted the default_value_lambdas branch May 29, 2020 20:23
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.

None yet

5 participants