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

Field Defaults: Allow default primitive w/o Lambda #191

Conversation

richardhsu
Copy link
Collaborator

  • [Update] Persistence was expecting the default value to support
    either call or dup but certain values like numbers and booleans
    do not support dup since they are copy by value. Adding support
    so can write :default => 1 or { default: 1 } as README
    suggests with field :actions_taken, :integer, {default: 0}.

CR @pboling @andrykonchin thanks!

- [Update] Persistence was expecting the default value to support
  either `call` or `dup` but certain values like numbers and booleans
  do not support `dup` since they are copy by value. Adding support
  so can write `:default => 1` or `{ default: 1 }` as README
  suggests with `field :actions_taken, :integer, {default: 0}`.
@coveralls
Copy link

coveralls commented Sep 13, 2017

Coverage Status

Coverage increased (+0.007%) to 96.865% when pulling 4b42392 on richardhsu:richardhsu/persistence-default-value-without-lambda into a33de98 on Dynamoid:master.

@coveralls
Copy link

coveralls commented Sep 13, 2017

Coverage Status

Coverage decreased (-0.3%) to 96.535% when pulling c211cc2 on richardhsu:richardhsu/persistence-default-value-without-lambda into a33de98 on Dynamoid:master.

@coveralls
Copy link

coveralls commented Sep 14, 2017

Coverage Status

Coverage increased (+0.007%) to 96.865% when pulling 8bebbfd on richardhsu:richardhsu/persistence-default-value-without-lambda into a33de98 on Dynamoid:master.

5 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 96.865% when pulling 8bebbfd on richardhsu:richardhsu/persistence-default-value-without-lambda into a33de98 on Dynamoid:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 96.865% when pulling 8bebbfd on richardhsu:richardhsu/persistence-default-value-without-lambda into a33de98 on Dynamoid:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 96.865% when pulling 8bebbfd on richardhsu:richardhsu/persistence-default-value-without-lambda into a33de98 on Dynamoid:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 96.865% when pulling 8bebbfd on richardhsu:richardhsu/persistence-default-value-without-lambda into a33de98 on Dynamoid:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 96.865% when pulling 8bebbfd on richardhsu:richardhsu/persistence-default-value-without-lambda into a33de98 on Dynamoid:master.

Copy link
Member

@pboling pboling left a comment

Choose a reason for hiding this comment

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

Excellent.

@pboling pboling merged commit a24de49 into Dynamoid:master Sep 14, 2017
@pboling
Copy link
Member

pboling commented Sep 14, 2017

@richardhsu @andrykonchin We should start adding to a HEAD section of a changelog with every PR so that we don't have so much work to do when preparing for the next release. Determine if the changes are breaking, or backwards compatible, new feature or bug fix, and add a note detailing the change to the CHANGELOG.

@richardhsu richardhsu deleted the richardhsu/persistence-default-value-without-lambda branch September 14, 2017 05:08
@richardhsu
Copy link
Collaborator Author

Oh, yes sounds good! Will do that moving forward! :)

@andrykonchin
Copy link
Member

@pboling please wait for my review before merging. I need at least a day to look at PR because of time zone shift

@andrykonchin
Copy link
Member

@richardhsu 👍

@pboling
Copy link
Member

pboling commented Sep 15, 2017

@andrykonchin OK, will wait in the future.

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

4 participants