-
Notifications
You must be signed in to change notification settings - Fork 11.6k
[5.6] Make migration from convenient naming conventions #20760
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
Conversation
I love how you snuck the StudlyCase thing into this PR. 👍 all around. |
👍🏻 |
|
||
/** | ||
* Ensure that a migration with the given name doesn't already exist. | ||
* Ensure that a migration with the given name does not already exist. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because typically contractions are used in conversational English and avoided in formal writing. But it's not a big deal, seems like a nit-picky thing to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never met a technical writer that dislikes contractions being used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if that were the case, why change it in this PR? Make a separate PR for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
He wrote and rewrote a bunch of comments in this PR. Are you seriously that offended by it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not saying it's right or wrong. Nor that it needs to be changed or not. I was simply adding to the idea of "formal writing" that I have yet (in like 6 years of working with technical writers) met one that dislikes their use. Which means, it can't be that informal if few who write professionally in the tech industry seems to really care in either direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not offended by anything? I'm just saying it's a useless change and not really in the scope of this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a trivial change. Honestly one that was either automated by my IDE or I did inadvertently when reviewing comments. Glad to revert it if additional changes are required for this PR.
Would it be ok if I created some code for renaming/dropping/etc foreign keys and indexes? I have hundreds of migrations that do that and having something like remove_fk_AAA_from_BBB and rename_index_999_to_index_888_in_XXX would be nice. |
@fabiolrs For sure. However, given the delay in merging this - I may break this out into its own package. If this gets merged into core, great. If not, then a package seems a good direction. |
I don't think I want to maintain this in the core. |
@jasonmccreary Did you end up creating that package? |
It would seem these new additions have ended up in no mans land. To @taylorotwell's point, they understandably bring too much overhead to core. However, given their nature - leveraging the core I may see if they are welcome additions to @JeffreyWay's generators package. Either way, I have opened #22648 with hopes the |
This expands upon the recently added naming convention of
create_XXX_table
by also allowing the following convenient naming conventions:The
_table
suffix is optional. If provided it will remain in the migration and class names, but automatically removed from the table name.I'll admit this implementation is naive. I took care to minimize impact on existing classes and methods. However, if well received by the community, the code could be greatly streamlined by removing the
table
andcreate
options and refining these conventions to support column data types.This also includes a small change to allow the developer to use a
StudlyCase
migration name in an effort to unify conventions with other artisan commands.