Closed AWolf81 closed 5 years ago
Merging #292 into master will increase coverage by
0.44%
. The diff coverage is51.42%
.
@@ Coverage Diff @@
## master #292 +/- ##
==========================================
+ Coverage 20.09% 20.54% +0.44%
==========================================
Files 235 237 +2
Lines 3648 3690 +42
Branches 368 369 +1
==========================================
+ Hits 733 758 +25
- Misses 2648 2667 +19
+ Partials 267 265 -2
Impacted Files | Coverage Δ | |
---|---|---|
src/config/app.js | 100% <ø> (ø) |
:arrow_up: |
src/components/CreateNewProjectWizard/MainPane.js | 0% <0%> (ø) |
:arrow_up: |
src/services/dependencies.service.js | 25% <0%> (-1.32%) |
:arrow_down: |
...rc/components/TaskDetailsModal/TaskDetailsModal.js | 0% <0%> (ø) |
:arrow_up: |
...nts/DevelopmentServerPane/DevelopmentServerPane.js | 0% <0%> (ø) |
:arrow_up: |
src/services/project-type-specifics.js | 83.33% <0%> (-16.67%) |
:arrow_down: |
src/components/TaskRunnerPane/TaskRunnerPane.js | 0% <0%> (ø) |
:arrow_up: |
src/services/process-logger.service.js | 33.33% <33.33%> (ø) |
|
src/actions/index.js | 93.39% <50%> (-0.89%) |
:arrow_down: |
src/reducers/tasks.reducer.js | 61.46% <58.82%> (+1.46%) |
:arrow_up: |
... and 5 more |
@melanieseltzer I think I've addressed your review changes. Could you please have a look?
The only open things are:
start
script is a problem as it's not detected as long running (see separate issue)I think we can tackle the open stuff afterwards.
Related Issue:
59
Summary: This branch is based on branch
nextjs-flow
(flow for workflow). I first started with this and I pushed it just for completeness. After merging the refactor branch we can safely delete the other branch. I've merged theprocess-logging
branch into this branch so it was easier to develop the refactoring.src/config/project-types.js
which contains three objects for storing the following information:isDevServerTask
,getDevServerTaskForProjectId
&getTaskType
to use the configuration object.As mentioned in the issue comment I first had the idea of having a function inside of the configuration object but I don't like that. Now I'm using the string
$port
inside of the configuration that will be replaced with the real port number in code. Just top-level replacement of$port
is not working but I think we could add this later. Also not sure if we need to support deeply nesting as it is only working for one level e.g.Will be replaced to (if available port is 3000):
I think this is also the first step for improving the Gatsby workflow. Do we have an issue for Gatsby improvement? I think I read that on Gitter but I'm not sure what should be improved.
Things to improve / discuss:
task.saga
line 179 addsadditionalArgs
to the command if test task running. We could also add this to the configuration but I think we can modify this later.Todo
create-next-app
because with-out my fork it wasn't working with the projectDirectory passed to the scriptnpx create-next-app c:\users\alexander\Guppy-dev\test
wasn't working. Maybe I can create a PR there if no other solution is available. OK, added a comment herePackage.json
of Next.js project and why there isstart
task missing. There is astart
task but it's not visible in Guppy as Melanie mentioned. Checked start script Seems like it requires to run build before. Sodev
is the right command to start the development server.Start
is not displayed as we're only filtering bytask name
so it gets removed from tasks list. I'll update this so we're more specific e.g.start
&create-react-app
. OK, I've modified this so the filtering is working but there is another issue becausestart
from next.js is also a long-running script which launches a local server we're having trouble with it. I'll open an issue for it so I can describe it in detail.Screenshots/GIFs: Project create with Next.js
Next.js project (failed build because I tested task stopping.)