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

[legacy-framework] Load env vars for blitz new command - ignore cached variables #3043

Merged
merged 3 commits into from
Dec 9, 2021

Conversation

beerose
Copy link
Contributor

@beerose beerose commented Dec 8, 2021

Closes: blitz-js/legacy-framework#370

What are the changes and their implications?

Not sure if that would be the best way to handle that (I might have missed something), so would love some suggestions if it's not.

Bug Checklist

  • Integration test added (see test docs if needed)

Feature Checklist

kodiakhq[bot]
kodiakhq bot previously approved these changes Dec 8, 2021
Comment on lines 70 to 71
dev?: boolean,
log: Log = console
log: Log = console,
ignoreCache = false
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove dev?

Copy link
Member

Choose a reason for hiding this comment

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

So ignoreCache is what fixes the issue right? Makes sense.

It would be slightly better to add an object as the new param as {ignoreCache}. I always fine unnamed function params error prone, especially when we are in a fork situation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why did you remove dev?

Not used in the function body

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change was introduced here: 5d9fcd2

Copy link
Member

Choose a reason for hiding this comment

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

Not used in the function body

Normally I would agree, but in the context of a fork, this makes upstream merges more difficult

@beerose beerose merged commit 0715bb7 into canary Dec 9, 2021
@beerose beerose deleted the fix-sqlite-init branch December 9, 2021 18:30
@itsdillon itsdillon changed the title Load env vars for blitz new command - ignore cached variables [legacy-framework] Load env vars for blitz new command - ignore cached variables Jul 7, 2022
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.

Blitz new command won't generate a SQLite database
3 participants