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

feat(template): adding meson-python as a build-system option #139

Merged
merged 12 commits into from
Jul 2, 2023

Conversation

YurelyCamacho
Copy link
Collaborator

Pull Request description

  • Activate option in TUI to meson-python.
  • Adding meson-python as a build-system:
    • Creating a mesonpy-pyproject.toml
    • Editing post_gen_project.py
    • Adding mesonpy to build-system.sh
    • Adding meson.build as additional configuration file
    • Editing cookicutter.json
    • Adding meson-python in conda/dev.yaml

Solve #55

Pull Request checklists

This PR is a:

  • bug-fix
  • [x ] new feature
  • [x ] maintenance

About this PR:

  • it includes tests.
  • the tests are executed on CI.
  • the tests generate log file(s) (path).
  • [ x] pre-commit hooks were executed locally.
  • [ x] this PR requires a project documentation update.

Author's checklist:

  • I have reviewed the changes and it contains no misspelling.
  • The code is well commented, especially in the parts that contain more complexity.
  • New and old tests passed locally.

Reviewer's Checklist

  • I managed to reproduce the problem locally from the main branch
  • I managed to test the new changes locally
  • I confirm that the issues mentioned were fixed/resolved .

@YurelyCamacho
Copy link
Collaborator Author

@xmnlab I have created this PR by adding the meson-python option as build system to see what happens with the tests. I still need to add the documentation, I will do it when I'm sure that everything else is working fine. Thanks

@YurelyCamacho YurelyCamacho requested a review from xmnlab June 28, 2023 20:02
Copy link
Member

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

@YurelyCamacho thanks for working on that.
it is looking great.

just a small comment inline here

I hope to finish my PR fixing some general issues with the smoke test
and I will ping you to ask you to rebase your code.

@YurelyCamacho YurelyCamacho requested a review from xmnlab June 28, 2023 21:15
Copy link
Member

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

@YurelyCamacho looks good to me.
could you rebase your branch on top of the upstream/main please?
just merged the PR fixing some issues with the smoke test.
also, as you mentioned on discord, could you ensure that these tests are being executed on CI please?
thanks!

@YurelyCamacho YurelyCamacho requested a review from xmnlab June 29, 2023 00:02
@xmnlab
Copy link
Member

xmnlab commented Jun 29, 2023

@YurelyCamacho it seems your PR included changes from my last PR, and now it is blocking with some git conflicts.
let me know if you need any help to fix that

@xmnlab
Copy link
Member

xmnlab commented Jun 29, 2023

@YurelyCamacho in general looks good. could you add the build-system smoke to be executed on CI please?

@YurelyCamacho
Copy link
Collaborator Author

@xmnlab Didn't run build-system test, error with flit. What can I do now?
Another question: should I make any changes to the last PR that was accepted?

@@ -44,6 +45,8 @@ if command -v poetry &> /dev/null; then
poetry install
elif command -v flit &> /dev/null; then
flit install
elif command -v mesonpy &> /dev/null; then
meson install
Copy link
Member

Choose a reason for hiding this comment

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

maybe the command would be pip install . instead

@@ -44,6 +45,8 @@ if command -v poetry &> /dev/null; then
poetry install
elif command -v flit &> /dev/null; then
flit install
elif command -v mesonpy &> /dev/null; then
Copy link
Member

Choose a reason for hiding this comment

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

maybe it would be meson instead of meonspy

@YurelyCamacho
Copy link
Collaborator Author

YurelyCamacho commented Jun 30, 2023

@xmnlab It is a problem with the installation of the package. It could be something with the pip install . command. It could be pip install -e .

@YurelyCamacho
Copy link
Collaborator Author

@YurelyCamacho YurelyCamacho marked this pull request as ready for review June 30, 2023 19:27
@YurelyCamacho
Copy link
Collaborator Author

I was looking at this to see if it solved the error. https://mkdocstrings.github.io/python/usage/

@xmnlab
Copy link
Member

xmnlab commented Jul 2, 2023

@YurelyCamacho good job! it seems that worked! congrats :)
LGTM!

@xmnlab xmnlab merged commit c05dc79 into osl-incubator:main Jul 2, 2023
@github-actions
Copy link

github-actions bot commented Jul 5, 2023

🎉 This PR is included in version 0.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants