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

Do not use --skip-update when running Kolibri in development mode #7938

Merged

Conversation

jonboiser
Copy link
Contributor

@jonboiser jonboiser commented Mar 31, 2021

Summary

Changes the devserver npm scripts to use the python-devserver script, which invokes kolibri start without the --skip-update flag.

This means, when you start Kolibri after deleting your KOLIBRI_HOME folder, or updating the KOLIBRI_HOME variable, etc, running the devserver for the first time will create the dev database and run the migrations first.

This also uses the run-p shorthand to replace npm-run-all --parallel in the 4 scripts

I also updated the "Getting started" guide to explain this change to the onboarding experience as well as the KOLIBRI_HOME env variable.

CleanShot 2021-03-30 at 18 19 43
CleanShot 2021-03-30 at 18 17 40

Reviewer guidance

Check to see that this new script works in different common situations like:

  1. Running Kolibri for the first time
  2. Running Kolibri for a second time
  3. Simulating an upgrade or a downgrade and running the devserver script.

References

Fixes #7302

Contributor Checklist

PR process:

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Testing:

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

Reviewer Checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@jonboiser jonboiser added this to the 0.15.0 milestone Mar 31, 2021
@jonboiser jonboiser requested a review from rtibbles March 31, 2021 00:48
@jonboiser jonboiser force-pushed the simplify-devserver-start branch from 9c4a63e to 2135cf9 Compare March 31, 2021 00:51
@rtibbles rtibbles added the TAG: developer docs Technical docs and code comments label Mar 31, 2021
@jonboiser
Copy link
Contributor Author

@rtibbles What needs to be updated in the dev docs? I can just add it to this PR

@rtibbles rtibbles removed the TAG: developer docs Technical docs and code comments label Mar 31, 2021
@rtibbles
Copy link
Member

@radinamatic had just added this: https://github.com/learningequality/kolibri/blob/develop/docs/getting_started.rst#database-setup because of this exact issue, but with this change it's now redundant :)

@rtibbles rtibbles added the TAG: developer docs Technical docs and code comments label Mar 31, 2021
@jonboiser jonboiser merged commit ea6dd6e into learningequality:develop Apr 1, 2021
@jonboiser jonboiser deleted the simplify-devserver-start branch April 1, 2021 17:07
@radinamatic
Copy link
Member

Oh, well... 🤷🏽‍♀️

Do we need to revert and delete the Database setup step now?

@jonboiser
Copy link
Contributor Author

Do we need to revert and delete the Database setup step now?

I removed that section, but added some explanation that the database setup happens as part of the first yarn run devserver command.

Hopefully, after this change. The onboarding experience is simple as 1. clone the repo, 2. install deps, 3. yarn run devserver

@rtibbles
Copy link
Member

rtibbles commented Apr 1, 2021

@jonboiser not seeing the docs updates in this PR, did you push them?

@jonboiser
Copy link
Contributor Author

Whoops, I did not!

@jonboiser
Copy link
Contributor Author

Here's the diff that I did not push 😭

jonboiser@23cbb34

I can revisit this after #7940 is merged, just to make sure the whole onboarding story is documented consistently.

@rtibbles
Copy link
Member

rtibbles commented Apr 1, 2021

Have merged #7940

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TAG: developer docs Technical docs and code comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants