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

🎨 grunt release .knex-migrator #7591

Merged

Conversation

kirrg001
Copy link
Contributor

@kirrg001 kirrg001 commented Oct 18, 2016

no issue

  • the file was not copied over for the release

These changes were required to create a successful build.
We will revert this change, when we have changed the filename.

no issue
- the file was not copied over for the release
- @todo: different solution?
@kirrg001 kirrg001 added this to the 1.0.0-alpha.6 milestone Oct 19, 2016
@ErisDS
Copy link
Member

ErisDS commented Oct 21, 2016

I think this is fine to merge for the next alpha.

My suggestion, long term, would be to change the filename. The name dotKnex-Migrator seems like a plain-text / yaml type config file, whereas in reality this is a javascript file. I think using a similar pattern to knex's own Knexfile.js might be clearer in indicating that it should be a JS file, and would also resolve the problem with dotfiles being ignored.

@ErisDS ErisDS mentioned this pull request Oct 21, 2016
4 tasks
@kirrg001
Copy link
Contributor Author

My suggestion, long term, would be to change the filename. The name dotKnex-Migrator seems like a plain-text / yaml type config file, whereas in reality this is a javascript file. I think using a similar pattern to knex's own Knexfile.js might be clearer in indicating that it should be a JS file, and would also resolve the problem with dotfiles being ignored.

Agreed. Will change the filename today. We can leave this PR open until we have a filename fix merged.

@kirrg001 kirrg001 changed the title [WIP] 🎨 grunt release .knex-migrator 🎨 grunt release .knex-migrator Oct 24, 2016
@ErisDS ErisDS merged commit c8c696f into TryGhost:master Oct 24, 2016
@kirrg001
Copy link
Contributor Author

Will revert this change when the knex-migrator filename change is merged 👍

mixonic pushed a commit to mixonic/Ghost that referenced this pull request Oct 28, 2016
no issue
- the file was not copied over for the release
- @todo: different solution?
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this pull request Nov 20, 2016
no issue
- the file was not copied over for the release
- @todo: different solution?
@ErisDS ErisDS mentioned this pull request Jan 17, 2017
22 tasks
@ErisDS ErisDS removed their assignment Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants