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 to specify a type for `references` in migrations #16231

Merged
merged 1 commit into from Jul 22, 2014

Conversation

@Envek
Copy link
Contributor

Envek commented Jul 19, 2014

This allows to specify a type for created foreign key column in references and add_reference in migrations, very handy:

def change
  enable_extension 'uuid-ossp'
  create_table :cars, id: :uuid do |t|
    t.integer :seats
    # And other car-specific things
  end
  create_table :wheels do |t|
    t.references :car, type: :uuid, index: true
    t.integer :radius
    # And other wheel-specific things
  end
end

Also I've changed PostgreSQL test to use this shiny functionality (looks better, eh?). And if anyone will ever remove this (not by reverting this PR), test will fail, showing that folks that use Postgres really need it ;-)

Bonus: documentation for references method.

P.S> After I wrote this PR I've found PR #13959 (which does the same, but little bit lengthy). Also see it for previous discussion. Choose either you like, I'll be happy with any of these PRs merged.

Will close #13959 if merged.

@senny senny changed the title Allow to specify a type for foreign key column in migrations Allow to specify a type for `references` in migrations Jul 21, 2014
@senny
senny reviewed Jul 22, 2014
View changes
activerecord/CHANGELOG.md Outdated
* Allow to specify a type for created foreign key column in `references` and
`add_reference` in migrations:

change_table :vehicle do |t|

This comment has been minimized.

Copy link
@senny

senny Jul 22, 2014

Member

can you add Example: above this line? See other entries for the general style.

@senny
senny reviewed Jul 22, 2014
View changes
activerecord/CHANGELOG.md Outdated
@@ -1,3 +1,12 @@
* Allow to specify a type for created foreign key column in `references` and
`add_reference` in migrations:

This comment has been minimized.

Copy link
@senny

senny Jul 22, 2014

Member

When we add Example: we should change : to a ..

@senny
senny reviewed Jul 22, 2014
View changes
activerecord/CHANGELOG.md Outdated
t.references :station, type: :uuid
end

*Andrey Novikov*

This comment has been minimized.

Copy link
@senny

senny Jul 22, 2014

Member

can you add the original author of the patch as well?

@senny
Copy link
Member

senny commented Jul 22, 2014

Can you add the following to the commit message:

[Łukasz Sarnacki & Novikov Andrey]

This will credit both authors for the patch.

assert column_exists?(table_name, :user_id, :string)
end

def test_does_not_create_reference_id_with_specified_type

This comment has been minimized.

Copy link
@senny

senny Jul 22, 2014

Member

Any specific reason for this test?

This comment has been minimized.

Copy link
@Envek

Envek Jul 22, 2014

Author Contributor

No, just followed other tests in this file…

@senny
Copy link
Member

senny commented Jul 22, 2014

@Envek added a couple minor comments. This patch looks good. Let me know when you updated the code.

[Andrey Novikov & Łukasz Sarnacki]
@Envek
Copy link
Contributor Author

Envek commented Jul 22, 2014

Updated code, squashed, and rebased on top of current master.

@senny senny merged commit 6d327db into rails:master Jul 22, 2014
1 check failed
1 check failed
continuous-integration/travis-ci The Travis CI build failed
Details
senny added a commit that referenced this pull request Jul 22, 2014
* Allow to specify a type for foreign key column in migrations
* unified the docs
* some cleanup in CHANGELOG
@senny
Copy link
Member

senny commented Jul 22, 2014

@Envek modified the docs and the CHANGELOG slightly.

@lukesarnacki , @Envek thank you for your work 💛

@Envek Envek deleted the Envek:type_in_references branch Jul 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.