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

Adopt monorepo structure #808

Merged
merged 12 commits into from
Jan 21, 2021
Merged

Adopt monorepo structure #808

merged 12 commits into from
Jan 21, 2021

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Jan 19, 2021

Adopt a monorepo structure with lerna to streamline the build process, and align the setup with current practices also used in other repos.

This makes it easier to build the packages with a single streamlined workflow:

jlpm
jlpm run build

This is in preparation for switching to TypeScript for the Voila frontend, and to the lab 3 based frontend later on.

TODO

  • Strict null checks
  • Lint check on CI
  • Simplify setup.py
  • Update contributing guide
  • Lint packages/voila

@jtpio jtpio marked this pull request as ready for review January 19, 2021 18:11
@jtpio
Copy link
Member Author

jtpio commented Jan 19, 2021

cc @martinRenou if you want to review, since you've also been looking at similar structures and build systems recently.

The idea would be to merge this PR first before migrating to TypeScript for the Voila frontend.

@martinRenou
Copy link
Member

Sorry I missed the notification, LGTM 👍🏼

@jtpio
Copy link
Member Author

jtpio commented Jan 21, 2021

Reverted back to the commit where CI was passing: 0108d27:

image

Need to investigate why the Voila tests suddenly started to fail now.

@jtpio jtpio marked this pull request as draft January 21, 2021 07:39
@jtpio
Copy link
Member Author

jtpio commented Jan 21, 2021

e0be580 should fix #755

@jtpio
Copy link
Member Author

jtpio commented Jan 21, 2021

Recreating an environment from scratch and rerunning the tests locally with python -m pytest seems to work fine:

image

However, running with VOILA_TEST_DEBUG=1 VOILA_TEST_XEUS_CLING=1 python -m pytest yields similar errors as on CI:

image

@jtpio
Copy link
Member Author

jtpio commented Jan 21, 2021

Looks like this might have to do with the recent 0.6.24 release of xtl published yesterday:

image

As downgrading to 0.6.23 seems to fix it:

voila-tests-xtl-0-6-24.mp4

@jtpio
Copy link
Member Author

jtpio commented Jan 21, 2021

Tests are green again with the pin to xtl=0.6.23 in 1372b45

@JohanMabille
Copy link
Member

I think it's fine to pin to xtl=0.6.23 for now. The xeus-cling stack must be rebuilt so it uses the same standard lib as the last xtl

@jtpio
Copy link
Member Author

jtpio commented Jan 21, 2021

I think it's fine to pin to xtl=0.6.23 for now. The xeus-cling stack must be rebuilt so it uses the same standard lib as the last xtl

OK sounds good, thanks for the heads-up 👍

We can open a new issue to remember to unpin at some point in the future.

@jtpio jtpio marked this pull request as ready for review January 21, 2021 13:45
@jtpio
Copy link
Member Author

jtpio commented Jan 21, 2021

Thanks Martin and Johan!

Looks good now, so we can merge and iterate in follow-up PRs.

@jtpio jtpio merged commit 3209c1f into voila-dashboards:master Jan 21, 2021
@jtpio jtpio deleted the monorepo branch January 21, 2021 14:23
@jtpio jtpio mentioned this pull request Jan 21, 2021
@jtpio
Copy link
Member Author

jtpio commented Jan 21, 2021

Opened #812 to keep track of the unpin.

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