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

More standard engine config #65

Merged
merged 5 commits into from
Oct 31, 2019

Conversation

ramontayag
Copy link
Contributor

@ramontayag ramontayag commented Apr 22, 2016

Makes the configuration of the gem a little bit more like the standard way. Main highlight:

Engine migration files are generated with rails g migration .... This allows for future changes to the db to be handled by Rails (detects which migrations are not there), so that the developer won't need to run special rails g plutus:upgrade commands.

Note: Keep tenant column in the dummy app. Until we merge the feature that always turns on tenancy
(#79), we must keep the dummy Rails app's migrations disjoint from what rake plutus:migrations:install actually copies over

@mbulat
Copy link
Owner

mbulat commented May 13, 2016

Which version of rails is this targeting? I think this supports 4.2.6 if we change the documentation for the migration command to rake plutus:install:migrations (i'm assuming it's a typo here, since #66 shows the rake command) Is the binstub required for 5.0? I'd like to make sure we target current stable (ie 4.2.6) until 5.0 comes out of RC.

Also, this is a breaking change for anyone seeking to upgrade from something like 0.8 to this version. I'm okay with that since we're not yet at 1.0. Maybe just a quick note in the readme along the lines of an upgrade path like 0.8 -> 0.12.2 -> 0.13 or something

Thanks!!

@ramontayag
Copy link
Contributor Author

ramontayag commented Jul 12, 2016

@mbulat sorry - I missed this somehow! The binstub is not required for Rails 5 - that's just for development. When the developer of the gem calls rails (assuming ./bin is in the path - I like to use direnv for this), they will be calling the right rails for the project.

Sure, I'll add the breaking change note.

@ramontayag ramontayag force-pushed the more_standard_engine_config branch from d1eefb3 to c1f28c0 Compare July 12, 2016 03:34
@ramontayag ramontayag mentioned this pull request Sep 1, 2016
1 task
So that we can generate migration files the way Rails guides suggests
@ramontayag ramontayag force-pushed the more_standard_engine_config branch from c1f28c0 to 28567e3 Compare July 9, 2018 07:42
@coveralls
Copy link

coveralls commented Jul 9, 2018

Coverage Status

Coverage increased (+0.02%) to 94.54% when pulling 56035ad on bloom-solutions:more_standard_engine_config into 00a98af on mbulat:master.

@ramontayag ramontayag force-pushed the more_standard_engine_config branch 2 times, most recently from 7c66bb0 to f15c00e Compare July 9, 2018 07:52
@ramontayag
Copy link
Contributor Author

@mbulat what do you think of this change?

@ramontayag ramontayag force-pushed the more_standard_engine_config branch from f15c00e to a2b678a Compare July 9, 2018 08:18
@ramontayag ramontayag changed the title More standard engine config WIP: More standard engine config Jul 9, 2018
Keep tenant column:

Until we merge the feature that always turns on tenancy
(mbulat#79), we must keep
the dummy Rails app's migrations disjoint from what
`rake plutus:migrations:install` actually copies over
@ramontayag ramontayag force-pushed the more_standard_engine_config branch from d3de0bf to b7524d0 Compare July 9, 2018 08:28
@ramontayag ramontayag changed the title WIP: More standard engine config More standard engine config Jul 9, 2018
@ramontayag ramontayag force-pushed the more_standard_engine_config branch from b7524d0 to 56035ad Compare July 9, 2018 08:36
dennyabraham added a commit to dennyabraham/tyche that referenced this pull request Jul 9, 2019
@mbulat mbulat merged commit aecd811 into mbulat:master Oct 31, 2019
@ramontayag ramontayag deleted the more_standard_engine_config branch November 16, 2020 07:25
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