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

refactor(gatsby): move redirects-writer to bootstrap #13008

Merged
merged 1 commit into from
Apr 3, 2019

Conversation

Moocar
Copy link
Contributor

@Moocar Moocar commented Apr 2, 2019

Description

In a future sub-PR of #13004, I'll be moving the query-runner from src/internal-plugins/query-runner to src/query. I was surprised to find that redirects-writer was part of query-runner. I think it makes more sense to be under bootstrap so have moved it.

Related Issues

@Moocar Moocar requested review from a team as code owners April 2, 2019 00:15
@Moocar Moocar changed the title move redirects-writer to bootstrap refactor(gatsby): move redirects-writer to bootstrap Apr 2, 2019
@Moocar Moocar force-pushed the redirects-writer-move-to-bootstrap branch from 5ac9b70 to 61d7e5d Compare April 2, 2019 05:14
@wardpeet
Copy link
Contributor

wardpeet commented Apr 2, 2019

It's probably not worth it to tangle this in your PRs but would it be better to get the bootstrapFinished local variable out of redux store?

convert


into

store.getState().program.status === 'BOOTSTRAP_FINISHED'

Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

Thanks! not much to review 😛

I'm holding off on merges as I want @pieh to have a look as well.

@Moocar
Copy link
Contributor Author

Moocar commented Apr 2, 2019

would it be better to get the bootstrapFinished local variable out of redux store?

@wardpeet Absolutely, but I'd probably go a step further and explicitly start the debounced writer from develop.js. Like I just did for db.startAutosave(). I'll add a TODO to me list.

@wardpeet wardpeet assigned wardpeet and pieh and unassigned wardpeet Apr 2, 2019
@Moocar
Copy link
Contributor Author

Moocar commented Apr 2, 2019

to be clear, I'll refactor redirects-writer in a future PR. So this one should be good to merge.

@pieh pieh merged commit 2b08f6a into gatsbyjs:master Apr 3, 2019
@Moocar Moocar deleted the redirects-writer-move-to-bootstrap branch April 3, 2019 21:00
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.

3 participants