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

Accepting nil as enum value #1270

Merged
merged 4 commits into from
Aug 18, 2016
Merged

Accepting nil as enum value #1270

merged 4 commits into from
Aug 18, 2016

Conversation

ProGM
Copy link
Member

@ProGM ProGM commented Aug 15, 2016

Fixes #1261

This pull introduces/changes:

  • Adds a :_default option to define the default value for enums.

This is a braking change.
If we want to keep the previous behavior, we can implement an _allow_nil option, to enable nil as default value.

Pings:
@cheerfulstoic
@subvertallchris

@codecov-io
Copy link

codecov-io commented Aug 16, 2016

Current coverage is 97.26% (diff: 100%)

Merging #1270 into 7.0.x will increase coverage by <.01%

@@              7.0.x      #1270   diff @@
==========================================
  Files           168        168          
  Lines         10404      10418    +14   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          10119      10133    +14   
  Misses          285        285          
  Partials          0          0          

Powered by Codecov. Last update fd96dfa...818ba3b

@@ -29,7 +29,7 @@ A Neo4j OGM (Object-Graph-Mapper) for use in Ruby on Rails and Rack frameworks h
s.add_dependency('orm_adapter', '~> 0.5.0')
s.add_dependency('activemodel', '~> 4')
s.add_dependency('activesupport', '~> 4')
s.add_dependency('neo4j-core', '>= 6.0.0')
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe both >= 6.0.0 and < 7.0.0? Is that possible in a gemspec?

Copy link
Member Author

Choose a reason for hiding this comment

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

@cheerfulstoic Yup, it's possible ;)

@cheerfulstoic
Copy link
Contributor

I'm good with this! I hope that nobody is depending on the breaking change, but they may very well need the _default option for backwards compatibility until they can fix their app, so that's great to have there. If you merge I can release a patch and put out a warning on Gitter / Slack

@ProGM
Copy link
Member Author

ProGM commented Aug 16, 2016

Great!
I fixed the gemspec thing ;)

Dunno if it's ok to merge now, or just wait for the test here: #1261

@ProGM
Copy link
Member Author

ProGM commented Aug 17, 2016

@cheerfulstoic ruby 2.0.0 fails to bundle ._.

@ProGM
Copy link
Member Author

ProGM commented Aug 18, 2016

Fixed!
Merging ;)

@ProGM ProGM merged commit 2891b21 into 7.0.x Aug 18, 2016
@ProGM ProGM removed the in progress label Aug 18, 2016
@ProGM ProGM deleted the fix-enum-default-value branch August 18, 2016 14:10
cheerfulstoic added a commit that referenced this pull request Aug 18, 2016
ProGM added a commit that referenced this pull request Aug 21, 2016
* 7.1.x:
  Release .15 patch with #1270 change
  Trying to fix ruby 2.0 build.
  Fix gemspec.
  Fix Gemfile for jruby.
  Accepting `nil` as enum value
  Update ActiveRel.rst
  Up to 7.1.2 to include fix from model_label_map_fix
  Put this back, because of course it broke things
  Fix issue with label / model mappings getting stuck before all models have a chance to load.  Also don't clear WRAPPED_CLASSES (I think this will fix the CypherNode issue we've been seeing in development
ProGM added a commit that referenced this pull request Aug 21, 2016
* 8.0.x:
  CHANGELOG entry for #1254
  Improvements based on #1264
  Destroy should return object like in ActiveRecord
  Use ActiveSupport::Logger instead of base Logger
  Release .15 patch with #1270 change
  Trying to fix ruby 2.0 build.
  Fix gemspec.
  Fix Gemfile for jruby.
  Accepting `nil` as enum value
  Add information about which exceptions are replacing old exceptions
  Update ActiveRel.rst
  Up to 7.1.2 to include fix from model_label_map_fix
  Put this back, because of course it broke things
  Fix issue with label / model mappings getting stuck before all models have a chance to load.  Also don't clear WRAPPED_CLASSES (I think this will fix the CypherNode issue we've been seeing in development
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.

3 participants