Skip to content
This repository has been archived by the owner on Apr 6, 2022. It is now read-only.

Use -f nightly flag with web-ext #95

Merged
merged 2 commits into from Apr 22, 2020
Merged

Use -f nightly flag with web-ext #95

merged 2 commits into from Apr 22, 2020

Conversation

rehandalal
Copy link
Contributor

r?

@rehandalal rehandalal requested a review from mythmon April 22, 2020 22:01
Copy link
Contributor

@mythmon mythmon left a comment

Choose a reason for hiding this comment

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

This is correct, but now we don't need to run the two commands in separate terminals, as yarn watch should just work. I'm not sure if this a better workflow or not (I haven't tried it). It might be worth only mentioning yarn watch, or softening the language of running the command in separate terminals.

Or we could leave it as it is now, your call.

bors delegate+

@bors
Copy link
Contributor

bors bot commented Apr 22, 2020

✌️ rehandalal can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@rehandalal
Copy link
Contributor Author

I only use the yarn watch workflow and it works well for me. Although I think there's room for a little bit of improvement because it does a double build to start which is a little annoying.

Copy link
Contributor Author

@rehandalal rehandalal left a comment

Choose a reason for hiding this comment

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

bors r=@mythmon

@bors
Copy link
Contributor

bors bot commented Apr 22, 2020

Build succeeded:

@bors bors bot merged commit cc6b0ec into mozilla-extensions:master Apr 22, 2020
@mythmon
Copy link
Contributor

mythmon commented Apr 22, 2020

I only use the yarn watch workflow and it works well for me. Although I think there's room for a little bit of improvement because it does a double build to start which is a little annoying.

That's actually by design. webpack --watch will build do a build first, and then start watching. There isn't a way to wait for that first build to finish though, and we need to make sure there is some build before starting web-ext. So yarn watch runs a normal build first, and then starts both webpack and web-ext in watch mode in parallel.

There has been an open issue for Webpack for a long time to give it an option to watch but not build, and it would solve this exact case. There is even a PR for it, but it hasn't been merged.

Is the double build actually slowing you down, or just wasteful? I wonder if we could make the second build basically a no-op if we tweak the caching or some other options.

@rehandalal
Copy link
Contributor Author

It's not really slowing me down at most times. It's just if I'm making certain kind of changes like to the webpack config where I need to restart it can sometimes be annoying. I'm not too concerned about it.

@mythmon
Copy link
Contributor

mythmon commented Apr 22, 2020

Ah, ok. The second build doesn't block the extension starting, it's just noise. The first build, which is blocking in your case, is needed anyways. And watch mode rebuilds seem pretty quick to me. So the double build isn't making anything worse.

It sounds like there might be some small benefit in trying to optimize our build times, but it's not a big deal.

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

Successfully merging this pull request may close these issues.

None yet

2 participants