Skip to content
This repository has been archived by the owner on Oct 24, 2022. It is now read-only.

Adds topic selections for subscribers #153

Merged
merged 1 commit into from
Apr 3, 2015
Merged

Conversation

richardrodgers
Copy link
Collaborator

Fixes #151
Fixes #93

@coveralls
Copy link

Coverage Status

Coverage increased (+0.35%) to 25.52% when pulling 834d587 on 151_selections into 2c89e4f on master.

@JPrevost
Copy link
Member

JPrevost commented Apr 3, 2015

I'll have to refactor my PR to use a different evolution name. Shouldn't be a problem. I suspect this is why rails uses timestamps as part of a migration name instead of just an Integer.

@richardrodgers
Copy link
Collaborator Author

Ahh, sorry - I hadn't seen that I stomped on your evolution SQL

@richardrodgers
Copy link
Collaborator Author

We should also maybe open a ticket (if you agree) about coalescing all the evolutions (perhaps at each milestone) at some point.

import models.Channel
import models.Collection
import models.ContentType
import models.{Agent, Channel, Collection, ContentType}
Copy link
Member

Choose a reason for hiding this comment

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

Ah... I'll start doing this too. Listing one per line was annoying me as I was doing it :)

@JPrevost
Copy link
Member

JPrevost commented Apr 3, 2015

I'm not sure we should change the migrations after they are merged as it may lead to confusion as to what is applied where. I'm definitely fine with a ticket to consider it further though.

@JPrevost
Copy link
Member

JPrevost commented Apr 3, 2015

👍

@richardrodgers
Copy link
Collaborator Author

Sorry, I didn't mean to imply altering commits. Rather, I believe it is common to express the results of applying the migrations in a single DDL file (in rails, db/schema.rb, or db/structure.sql, is it?) that is in source control, and is the deployed file when installing. My question was whether we do that once at the end of the project, or have a m1.sql, m2.sql, etc

@JPrevost
Copy link
Member

JPrevost commented Apr 3, 2015

The schema.rb is indeed checked into source control in rails (generally), but usually the migrations are actually run as part of deploy. schema.rb only really comes into play when doing a rake db:reset which isn't a normal process when deploying as you'd lose all your data.

In essence, I think it would be nice if Play had something like schema.rb as it's convenient to look at for reference, but I think we should run the Evolutions as part of deploy as there isn't really any built in guarantee that the final state of the database as reflected by something like schema.rb actually matches the end results of all of the evolutions/migrations. The structure should end up the same, but technically you can insert data as part of evolutions/migrations that would not be reflected in something like schema.rb (although that's probably a bad idea).

@richardrodgers
Copy link
Collaborator Author

So you disagree with:

6.1 What are Schema Files for? Migrations, mighty as they may be, are not the authoritative source for your database schema. That role falls to either db/schema.rb or an SQL file which Active Record generates by examining the database. They are not designed to be edited, they just represent the current state of the database.

There is no need (and it is error prone) to deploy a new instance of an app by replaying the entire migration history. It is much simpler and faster to just load into the database a description of the current schema.

(just randomly googled: at http://edgeguides.rubyonrails.org/active_record_migrations.html)

Just don't know rails best practices, sorry to be dense

@JPrevost
Copy link
Member

JPrevost commented Apr 3, 2015

I honestly don't know how you'd use a schema file to selectively apply changes without losing the entire database in the process.

richardrodgers pushed a commit that referenced this pull request Apr 3, 2015
Adds topic selections for subscribers
@richardrodgers richardrodgers merged commit 652dc26 into master Apr 3, 2015
@richardrodgers richardrodgers deleted the 151_selections branch April 3, 2015 18:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide suggested topics for subscribers Cataloger ERROR: value too long for type character varying(255)
3 participants