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

In track_associations, only check db if no config #640

Merged
merged 1 commit into from
Nov 8, 2015
Merged

Conversation

jaredbeck
Copy link
Member

This allows users who need to precompile assets without a database
to configure track_associations = false.

See discussion: #636

This allows users who need to precompile assets without a database
to configure `track_associations = false`.

See discussion: #636
@track_associations ||= PaperTrail::VersionAssociation.table_exists?
@track_associations.nil? ?
PaperTrail::VersionAssociation.table_exists? :
@track_associations
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could actually simplify this to @track_associations && PaperTrail::VersionAssociation.table_exists?, right? Not as explicitly obvious what is going on, but my thought is that this will actually prevent errors in scenarios where users try to turn the feature on without having the underlying required table. Alternatively we could raise an error or a warning in those scenarios. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could actually simplify this to @track_associations && PaperTrail::VersionAssociation.table_exists?, right?

That conjunction would require a database connection, and thus would not fix #636

@batter
Copy link
Collaborator

batter commented Nov 8, 2015

Ahh I see

batter added a commit that referenced this pull request Nov 8, 2015
In track_associations, only check db if no config
@batter batter merged commit cd04d16 into master Nov 8, 2015
@batter batter deleted the fix_issue_636 branch November 8, 2015 21:25
@batter
Copy link
Collaborator

batter commented Nov 8, 2015

I'm thinking it may make sense to merge this upstream into the 4.0-stable branch and have it be merged into 4.0.1, since it is a bug fix. Thoughts?

@jaredbeck
Copy link
Member Author

I'm thinking it may make sense to merge this .. into the 4.0-stable branch and have it be merged into 4.0.1, since it is a bug fix.

Sounds good. I'll go ahead and do that merge if you haven't already, and I'll add a line to the changelog about it. What's your timeline for a 4.0.1 release, because I think we've done enough work to justify one, it's just a matter of tracking down what we need to backport.

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