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

Don't duplicate id/created_at/updated_at fields if they are passed in #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EspadaV8
Copy link
Contributor

The current migrations assume that everyone wants the default id and
timestamp definitions added, but this isn't always the case. It now
checks to see if any schema definitions have been passed in for id,
created_at and updated_at and if they have then the default fields
aren't added.

The current migrations assume that everyone wants the default id and
timestamp definitions added, but this isn't always the case. It now
checks to see if any schema definitions have been passed in for id,
created_at and updated_at and if they have then the default fields
aren't added.
@EspadaV8
Copy link
Contributor Author

My use case for this is using PostgreSQL. My default is to use uuid columns for ids and timestampTz for the created_at and updated_at columns.

The current behaviour remains for ids if the user doesn't pass in an id definition. The timestamps won't be added if either created_at or updated_at have been passed in (otherwise they'd be a column clash).

Tests have been added for the new functionality.

@tabacitu
Copy link
Contributor

tabacitu commented Mar 8, 2020

Thank you for taking the time to submit this @EspadaV8 ! Let me know if you're still using the package and would like to take a second look at this PR, before we merge it into v2 #170

This sounds like a great idea so I'll take another look at it after I get to the bottom of the PR&issues stack. It's finally time for this repo to get the attention it deserves. Personally I've been using it without adding improvements for YEARS now, I think it's finally time to bring it closer to its potential.

Let me know if you want to help out with another look at this, or with other features/improvements for v2 - we could use all the help we can get!

Cheers!

@EspadaV8
Copy link
Contributor Author

EspadaV8 commented Mar 8, 2020

Haha, yes, this is a bit old now. Looking at the code is still seems okay. Not sure what changes you have planned for the package now but feel free to merge this in if it's useful. I actually stopped using the package because of this (and other) issue(s) and ended up writing some custom Artisan commands with custom stub files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants