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

Use table name as part of database index name (#336) #340

Merged

Conversation

marksiemers
Copy link
Contributor

Description of the Change

Fixes #336

Alternate Designs

The lines that fix this issue exist in two ECR files. The code can likely be refactored into the Migration template class.

Benefits

Fixes #336

Possible Drawbacks

None known.

@marksiemers
Copy link
Contributor Author

I refactored to get the sql generation in one place (for the CREATE INDEX statements): 4145802

I can also refactor for the rest of the migration SQL generation in the PR if wanted.

LMK

@marksiemers
Copy link
Contributor Author

I have a separate branch and PR (in my fork) for the aforementioned refactor: https://github.com/marksiemers/amber/pull/1/files

It can be a separate PR, or I can merge so it goes with this change.

Copy link
Member

@drujensen drujensen left a comment

Choose a reason for hiding this comment

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

✨ 💯

@marksiemers
Copy link
Contributor Author

@drujensen - Do we want the additional refactor that is in my fork? Or should that be a seperate PR?
https://github.com/marksiemers/amber/pull/1/files

@drujensen
Copy link
Member

wow, nice cleanup work. It seems somewhat related so go ahead and include it. It doesn't look too big.

@eliasjpr eliasjpr merged commit 0c43a5c into amberframework:master Oct 31, 2017
elorest pushed a commit that referenced this pull request Nov 17, 2017
* Write tests for CREATE INDEX line in migrations template (#336)

* Implement including table name in CREATE INDEX line in migrations template

* Refactor generating CREATE INDEX sql statements for reference fields (#336)

* Refactor generating migrations

* Refactor tests for migration cli templates
elorest pushed a commit that referenced this pull request Nov 17, 2017
* Write tests for CREATE INDEX line in migrations template (#336)

* Implement including table name in CREATE INDEX line in migrations template

* Refactor generating CREATE INDEX sql statements for reference fields (#336)

* Refactor generating migrations

* Refactor tests for migration cli templates


Former-commit-id: fc551c5
@faustinoaq faustinoaq added this to Done in Framework 2018 May 5, 2018
@faustinoaq faustinoaq removed this from Done in Framework 2018 Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Index name needs to be unique
3 participants