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

fix pre-push hooks, and move to Poetry #2677

Merged

Conversation

joaoantoniocardoso
Copy link
Member

@joaoantoniocardoso joaoantoniocardoso commented Jun 11, 2024

This migrates the hooks/pre-push to use Poetry directly, and makes all the files contained within the project's virtual environment, instead of loading files on the users' folder, making the development environment more replicable.

To use the pre-push, the user must have a Python 3.11 available. Before, it would use whatever the system's Python was, failing to run when the user had 3.12 or 3.10 for example.

To test:

  • checkout to this PR's branch
  • clean the project: \rm -rf .venv poetry.lock
  • run the pre-push hook: .hooks/pre-push

To check whether it had any impact on the normal installation:

  • Try out the joaoantoniocardoso/blueos-core:fix-pre-push on BlueOS

I have tested it myself, but more samples are welcome.

After this patch is merged, we can all clean our system:

  • ls "$(python -m site --user-site)"
  • ls "$(python -m site --user-base)/bin"

@joaoantoniocardoso joaoantoniocardoso force-pushed the fix-pre-push branch 2 times, most recently from 38620f4 to faf8ae1 Compare June 11, 2024 18:24
@joaoantoniocardoso joaoantoniocardoso changed the title .hooks, core/tools/ardupilot_tools: Add NOSUDO env variable to skip sudo calls fix pre-push hooks, and move to Poetry Jun 11, 2024
@joaoantoniocardoso joaoantoniocardoso force-pushed the fix-pre-push branch 3 times, most recently from d061898 to 9bf0381 Compare June 12, 2024 00:35
@joaoantoniocardoso joaoantoniocardoso marked this pull request as draft June 12, 2024 16:12
@joaoantoniocardoso joaoantoniocardoso force-pushed the fix-pre-push branch 8 times, most recently from a37bb72 to 367375e Compare June 13, 2024 13:21
@joaoantoniocardoso joaoantoniocardoso marked this pull request as ready for review June 13, 2024 14:04
@joaoantoniocardoso joaoantoniocardoso marked this pull request as draft June 13, 2024 17:16
@joaoantoniocardoso joaoantoniocardoso marked this pull request as ready for review June 13, 2024 18:14
@Williangalvani
Copy link
Member

works great once it works! but I had to manually configure a local python3.11 with pyenv. can we add a check for python version and add hints on how to set it up?

  • install pyenv
  • run pyenv install 3.11 && pyenv local 3.11

@joaoantoniocardoso
Copy link
Member Author

@Williangalvani is the command python3.11 available when using pyenv, or should I use some pyenv command to figure it out?

@joaoantoniocardoso joaoantoniocardoso force-pushed the fix-pre-push branch 2 times, most recently from e6fe7ca to bb2c376 Compare June 15, 2024 15:08
@Williangalvani
Copy link
Member

@Williangalvani is the command python3.11 available when using pyenv, or should I use some pyenv command to figure it out?

once you do pyenv local 3.11, python3.11 is available on path

altenatively, pyenv versions show the versions available on your system:

➜  ~ pyenv versions
* system (set by /home/will/.pyenv/version)
  3.7.9
  3.8.18
  3.11.9

@joaoantoniocardoso
Copy link
Member Author

@Williangalvani

Okay, I rewrote the Python check code to automatically use pyenv if it's available

@Williangalvani
Copy link
Member

working well locally here

Copy link
Member

@Williangalvani Williangalvani left a comment

Choose a reason for hiding this comment

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

lgtm! this is so much better!

@joaoantoniocardoso
Copy link
Member Author

@Williangalvani I just rebased, some of the commits were already merged.

@joaoantoniocardoso joaoantoniocardoso merged commit 04d856e into bluerobotics:master Jun 15, 2024
7 checks passed
@joaoantoniocardoso joaoantoniocardoso deleted the fix-pre-push branch June 15, 2024 23:48
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.

None yet

2 participants