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

Add quick_setup.sh test to github action #1835

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

hunhoffe
Copy link
Collaborator

Test the quick_setup.sh environment/build using the programming examples and programming guide on Ryzen AI.

pip cache purge
source /opt/xilinx/xrt/setup.sh
python -m pip install virtualenv
Copy link
Collaborator

@fifield fifield Oct 14, 2024

Choose a reason for hiding this comment

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

Other workflows use venv. It is already installed, is part of python, and does not pip install anything to the github user's site packages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense to me! I've changed quick_setup to use venv instead of virtualenv.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense to me! I've changed quick_setup to use venv instead of virtualenv.

Note it was slightly religious statement I made (venv over virtualenv). There may be cases where virtualenv is less friction because ubuntu doesn't install venv by default and user might not have sudo to apt install it.

Copy link
Collaborator Author

@hunhoffe hunhoffe Oct 15, 2024

Choose a reason for hiding this comment

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

I don't have a strong preference either way (venv vs virtualenv), but I think it makes sense to be consistent. Is there anyone else who may want to weigh in? Otherwise, I'll stick to changes to venv.

@hunhoffe hunhoffe marked this pull request as ready for review October 15, 2024 20:05
@hunhoffe hunhoffe requested a review from fifield October 16, 2024 14: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