Navigation Menu

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 developer convenience scripts #1377

Merged
merged 9 commits into from Apr 15, 2020
Merged

Conversation

mweiden
Copy link
Contributor

@mweiden mweiden commented Apr 10, 2020

  • Move the backend-dev target in client/Makefile to its own script (scripts/backend_dev), thereby better decoupling the client/ from build targets elsewhere in the repo
  • Remove the client test target, this doesn't have much practical use anymore
  • Remove the backend-dev-anno-ontology target. Turning this on through the convenience scripts is still possible with CXG_OPTIONS, which you had to do anyway to run it through backend-dev
  • Add the scripts/frontend_dev convenience method, which opens up Terminal, starts the cellxgene in one tab on port 5005, and the node dev server on port 3000 for hot-reloading development
  • Update all the docs

frontend_dev is a soup-to-nuts convenience method for setting up the FE
development environment with node running a the client code on port 3000
with the a separate cellxgene package serving the API over port 5005 in
the background.

The script can be run from Finder.
@codecov-io
Copy link

codecov-io commented Apr 10, 2020

Codecov Report

Merging #1377 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1377   +/-   ##
=======================================
  Coverage   62.32%   62.32%           
=======================================
  Files          68       68           
  Lines        5274     5274           
  Branches      377      377           
=======================================
  Hits         3287     3287           
  Misses       1895     1895           
  Partials       92       92           
Flag Coverage Δ
#backend 52.73% <ø> (ø)
#frontend 75.26% <ø> (ø)
#javascript 75.26% <ø> (ø)
#python 52.73% <ø> (ø)
#smokeTest ?
#smokeTestAnnotations 100.00% <ø> (ø)
#unitTest 62.32% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14021db...25126c9. Read the comment docs.

@mweiden mweiden changed the title Refactor developer Refactor developer convenience scripts Apr 10, 2020
Rationale:
* Given how long the smoke tests take to run, it is unlikely that
  developers will want to run all tests together.
* It is unlikely that developers will have set up the backend server
  properly for the tests to pass.
* Available commands should be safe-ish and not lend themselves to
  confusing errors.
* You can still group tests by concatenating them in a make command, as
  in `make unit-test smoke-test`.
* This target isn't used in any of our CI pipelines -- KISS.
Copy link
Contributor

@bkmartinjr bkmartinjr left a comment

Choose a reason for hiding this comment

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

I like it. I'm not really the target audience though. If @seve and @colinmegill are good, you might also want to run it by @liaprins-czi

@seve
Copy link
Member

seve commented Apr 15, 2020

Is there a way to set a CXG_TERMINAL to change what terminal this opens up? Other than that LGTM

@mweiden
Copy link
Contributor Author

mweiden commented Apr 15, 2020

@seve I don't know of a way to generalize that yet, but will give it some thought and follow up with a second PR if I can find a solution.

What terminal are you using? iTerm2?

@mweiden mweiden merged commit 7d4d360 into master Apr 15, 2020
@mweiden mweiden deleted the mweiden/fe-dev-automation branch April 15, 2020 17:32
@seve
Copy link
Member

seve commented Apr 15, 2020

What terminal are you using? iTerm2?

I'm using Hyper(https://hyper.is/)

@mweiden
Copy link
Contributor Author

mweiden commented Apr 15, 2020

Hmmm, OK. Thinking.

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

4 participants