Skip to content

Provide :hide_default_column_types option to make hiding of default configurable #389

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

Merged
merged 2 commits into from
Jul 29, 2016
Merged

Provide :hide_default_column_types option to make hiding of default configurable #389

merged 2 commits into from
Jul 29, 2016

Conversation

jastkand
Copy link
Contributor

First of all, thanks for the great gem!

2.7.0 release made hidden the default value of hstore, json and jsonb columns. It's reasonable in such cases like this: #320. But that's not good when the default value is more simple, like default({}) or default([]). I think it would be great to have hiding of default configurable. This pull-request introduces this feature.

@coveralls
Copy link

coveralls commented Jul 18, 2016

Coverage Status

Changes Unknown when pulling bd910b0 on jastkand:configurable-hiding-of-default into * on ctran:develop*.

@ctran
Copy link
Owner

ctran commented Jul 18, 2016

Great! One question though, I didn't see the existing defaults of "json,jsonb,hstore" being used if the option is not specified. Did I miss it somewhere?

@jastkand
Copy link
Contributor Author

jastkand commented Jul 19, 2016

The goal was to be able to disable hiding of default. Using NO_LIMIT_COL_TYPES constant when the option value is blank does not allow to switch hiding off. In the previous implementation, the user had to explicitly specify "json,jsonb,hstore" which was not good.

Now I've updated the code to have the same behavior as hide_limit_column_types. Now in order to prevent hiding of default one can set the value of hide_default_column_types option to some value which does not represent the column type, like skip.

@coveralls
Copy link

coveralls commented Jul 19, 2016

Coverage Status

Changes Unknown when pulling dbf19d7 on jastkand:configurable-hiding-of-default into * on ctran:develop*.

@coveralls
Copy link

coveralls commented Jul 19, 2016

Coverage Status

Changes Unknown when pulling e007765 on jastkand:configurable-hiding-of-default into * on ctran:develop*.

@jastkand
Copy link
Contributor Author

@ctran Would you mind taking a look at the PR?

@ctran ctran merged commit 89d513c into ctran:develop Jul 29, 2016
@ctran
Copy link
Owner

ctran commented Jul 29, 2016

Thanks!!!

@ctran ctran added the feature label Jul 29, 2016
@ctran ctran added this to the v2.8.0 milestone Jul 29, 2016
@ctran ctran modified the milestones: v2.7.2, v2.8.0 Dec 17, 2016
@nneal
Copy link

nneal commented Jan 27, 2017

I'm trying to use this in our environment, but doesn't seem to be working.

I have hide_default_column_types' => 'skip', as a setting in the annotate config file, i'm invoking annotate with b exec rake annotate_models and still not showing defaults for us :\ Not sure where I'm going wrong

@jastkand
Copy link
Contributor Author

@nneal do you use version of gem from master? The feature is not yes released in rubygems version of gem.

@nneal
Copy link

nneal commented Jan 27, 2017

I am using gem 'annotate', git: 'https://github.com/ctran/annotate_models.git', branch: 'develop' because it appears to be the most up to date branch on github, but i also tried branch: 'master'. Neither worked for these settings :\ Thank you for the help

@nneal
Copy link

nneal commented Jan 27, 2017

@jastkand I left more information in here: #441

johncarney pushed a commit to johncarney/annotate_models that referenced this pull request Jun 19, 2017
…onfigurable (ctran#389)

* Provide :hide_default_column_types option to make hiding of default configurable

* Use `NO_DEFAULT_COL_TYPES` as default when the `hide_default_column_types` option is not provided
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants