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

Allow incompatible versions of ActiveRecord #1216

Merged
merged 1 commit into from
Jul 31, 2019

Conversation

jaredbeck
Copy link
Member

Re: #1213

For advanced users only. A warning is produced. See discussion
in paper_trail/compatibility.rb

  • Wrote good commit messages.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new
    code introduces user-observable changes.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.

For advanced users only. A warning is produced. See discussion
in paper_trail/compatibility.rb
@jaredbeck jaredbeck force-pushed the allow_incompatible_activerecord branch from 1b046a0 to a107146 Compare July 29, 2019 15:59
@jaredbeck
Copy link
Member Author

@rafaelfranca does this work for you?

@Edouard-chin
Copy link
Contributor

Thanks for looking into this @jaredbeck , much appreciated 🙇 .

I think outputting a warning each time the gem is loaded is not very user friendly (even if we can disable it with a special env). It would be better if the warning was output if papertrail get installed for the first time or whenever activerecord gets updated through a bundler update.

We can achieve that by using a small rubygem plugin and the Gem.post_install hook https://github.com/Edouard-chin/paper_trail/pull/new/ec-allow-incompatible-ar

Copy link
Contributor

@rafaelfranca rafaelfranca left a comment

Choose a reason for hiding this comment

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

I'm very happy with this change.

@jaredbeck
Copy link
Member Author

Thanks for the reviews.

It would be better if the warning was output if papertrail get installed for the first time or whenever activerecord gets updated through a bundler update.

Interesting idea, Edouard, but I'm not comfortable with it, sorry. Once we give up the safety net of an upper-bound, normal users will accidentally install incompatible versions. It will happen frequently. I want to be certain that the warning is seen. We have to take care of beginners and normal users.

@jaredbeck jaredbeck merged commit 3127d71 into master Jul 31, 2019
@jaredbeck jaredbeck deleted the allow_incompatible_activerecord branch July 31, 2019 20:36
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