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.

Sign up for GitHub

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

Jump to bottom

Split dependencies #1745

Merged
merged 7 commits into from
Sep 28, 2021
Merged

Split dependencies #1745

merged 7 commits into from
Sep 28, 2021

Conversation

anders-kiaer
Copy link
Contributor

@anders-kiaer anders-kiaer commented Sep 4, 2021 •
edited
Loading

Closes #1742.

Contributor Checklist

  • I have broken down my PR scope into the following TODO tasks
    • I have checked locally which dependencies are needed in order to run dash-generate-components, and as far as I could see only PyYAML, fire and coloredlogs are needed in dash[dev].
    • Moved dependencies that are used only by Dash CI/tests into requires-ci.txt.
    • Created two new "extra groups" for the long callback managers. This should make it easier for dash to potentially change which dependencies the callback managers require in the future, as well as communicate how to install what is needed.

@anders-kiaer anders-kiaer force-pushed the split_dep branch 5 times, most recently from bb7ad85 to 30dfc16 Compare September 7, 2021 09:19
@anders-kiaer anders-kiaer marked this pull request as ready for review September 7, 2021 09:44
@anders-kiaer
Copy link
Contributor Author

anders-kiaer commented Sep 7, 2021 •
edited
Loading

I'm a bit unsure if my .circleci/config.yml changes are optimal wrt. caching (not experienced with CircleCI, even though it has similarities with GitHub workflows). Also not sure why the Percy test fails. 😆

Splitting out Dash CI dependencies will ensure projects using dash[dev] and dash[testing] pull downs only necessary requirements, which again will reduce installation time and improve security for downstream projects.

alexcjohnson
alexcjohnson reviewed Sep 21, 2021
View reviewed changes
setup.py Outdated Show resolved Hide resolved
@anders-kiaer anders-kiaer force-pushed the split_dep branch 2 times, most recently from 6512378 to e644f60 Compare September 21, 2021 16:00
alexcjohnson
alexcjohnson approved these changes Sep 28, 2021
View reviewed changes
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 Thanks @anders-kiaer !

@alexcjohnson alexcjohnson merged commit 8c193bc into plotly:dev Sep 28, 2021
@anders-kiaer
Copy link
Contributor Author

Thanks @alexcjohnson for taking it over the 🏁 line. 🙂

@anders-kiaer anders-kiaer deleted the split_dep branch September 28, 2021 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonmmease jonmmease jonmmease left review comments

@alexcjohnson alexcjohnson alexcjohnson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Split out Dash CI only requirements into requires-ci.txt?
3 participants
@anders-kiaer @alexcjohnson @jonmmease