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

CI tool: Remove hardcoded version #20

Merged
merged 1 commit into from
Feb 16, 2021
Merged

CI tool: Remove hardcoded version #20

merged 1 commit into from
Feb 16, 2021

Conversation

manuq
Copy link
Collaborator

@manuq manuq commented Feb 15, 2021

From the github-actions configuration.

https://phabricator.endlessm.com/T31414

@manuq manuq requested a review from danigm February 15, 2021 19:32
tox.ini Outdated
@@ -65,7 +65,7 @@ commands =
yarn
make dist
# Test whl installation
pip install {toxinidir}/dist/kolibri_explore_plugin-0.0.1-py2.py3-none-any.whl
pip install {toxinidir}/dist/kolibri_explore_plugin-*.whl
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not working, I think it's because the tox tooling that sees that like a filename and doesn't apply the glob. Maybe we should add the version here like a variable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah. I don't know about tox but I will check how to do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is interesting: tox-dev/tox#1571

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's too complex we can just remove this line from tests, it's something not required to test. With the make dist is enough.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed. I updated the PR disabling the command and referencing the ticket in tox.

From the github-actions configuration.

We are disabling this command for now, since tox.ini doesn't
support expanding globs. tox-dev/tox#1571

https://phabricator.endlessm.com/T31414
@manuq manuq merged commit 8fb7cc5 into master Feb 16, 2021
@manuq manuq deleted the T31414-fix-ci branch February 16, 2021 13:58
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.

2 participants