-
Notifications
You must be signed in to change notification settings - Fork 18
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(file): add example code for the cli option defined by the user #225
Conversation
@YurelyCamacho it seems your branch is out-of-date |
8788169
to
83124c7
Compare
src/scicookie/{{cookiecutter.project_slug}}/build-system/poetry-pyproject.toml
Outdated
Show resolved
Hide resolved
src/scicookie/{{cookiecutter.project_slug}}/{{cookiecutter.package_slug}}/cli.py
Outdated
Show resolved
Hide resolved
src/scicookie/{{cookiecutter.project_slug}}/{{cookiecutter.package_slug}}/cli.py
Outdated
Show resolved
Hide resolved
src/scicookie/{{cookiecutter.project_slug}}/{{cookiecutter.package_slug}}/cli.py
Outdated
Show resolved
Hide resolved
@YurelyCamacho in gneral, it looks good to me. you also need to to update the CI file and makim in order to include the new test |
a17e8ff
to
69ab191
Compare
src/scicookie/{{cookiecutter.project_slug}}/{{cookiecutter.package_slug}}/cli.py
Outdated
Show resolved
Hide resolved
|
||
import argparse | ||
import os | ||
import sys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xmnlab I don't know why this error occurred https://github.com/osl-incubator/scicookie/actions/runs/8113662218/job/22177467364?pr=225#step:5:376 |
this problem could happen because some parallelized usage of the ruff cache, IIRC for now, don't worry about that, it seems that it still has an ruff issue in some file there, https://github.com/osl-incubator/scicookie/actions/runs/8113662218/job/22177467364?pr=225#step:5:377 try to fix that first, and maybe it will fix the first issue as well, of course, it would be better to have a more robust alternative to fix that ... I will try to find the configuration I saw that avoid this data racing from pre-commit |
please add to the ruff config in the pre-commit-config.yaml
this PR adds some information: pre-commit/pre-commit#2437 |
In ruff check or ruff format? |
this would be in the pre-commit hook config in the template folder: https://github.com/osl-incubator/scicookie/blob/main/src/scicookie/%7B%7Bcookiecutter.project_slug%7D%7D/.pre-commit-config.yaml#L43 there we just have for now 1 ruff command, in the future we should have both the formatter and the linter there as well |
Sorry, I was looking at the project's repo. |
@xmnlab I don't know what can solve this error, any suggestions? |
in the base.sh file, you can stop the script with "exit 1" before the let me know if you need more information |
Yes, I don't really understand what to do |
so .. you need to debug it, so how you can do it? you are executing a script .. so you need to break the script in a place where you can check the results and see what is happening. where is the step where it is failing? check on the log file first to identify what is the command in the base.sh that is failing. this is really important step please take your time to identify that, every time that the CI fails you need to identify where the error is happening after you identify that, you will find that it is happening here (please just move to this step when you really find the error in the step before) https://github.com/osl-incubator/scicookie/blob/main/tests/smoke/base.sh#L61 so before that you will need to stop the execution so you can do it by yourself and check the results. why do you need to step the execution? so the linter change the files in the (new) project folder (at /tmp/osl/osl-python-package) and you will execute the command by yourself and check the results and check what is happening. to stop the execution, just add a the instruction (exit 1) before the command that is failing now execute the specific test that is failing, if you have doubts, check the logs and try to find which test is failing, TIP: in the logs, when a line starts with so after you execute the specif test that is failing, it will break in the point you added now go to the new project created at does it make more sense now? let me know if that helped. |
201d3ee
to
2f9297a
Compare
@xmnlab Done, please review. It remains to create a new section in the documentation on how to debug the tests with the information you mentioned. |
src/scicookie/{{cookiecutter.project_slug}}/{{cookiecutter.package_slug}}/cli.py
Show resolved
Hide resolved
import click | ||
|
||
from {{ cookiecutter.package_slug }} import __version__ | ||
{%- if cookiecutter.use_black == "yes" %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain a bit this black specific workflow?
As far as I understood, this is used by to add 2 empty lines, correct? If so, I guess that could be used also when black is not used.
Could you give more details about tgat please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I did it this way because it gave me a black error because I didn't have the 2 empty lines. Let me test if I remove the black condition, how the tests behave.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xmnlab It worked without the condition for black. I was trying several things here until it worked. Can you check again? Thanks
@xmnlab Did you check that I removed the condition for black? I don't understand what is still missing? |
Co-authored-by: Ivan Ogasawara <ivan.ogasawara@gmail.com>
5d86217
to
1c33c5d
Compare
@xmnlab That's the change you told me about, right? |
yep .. that is what I was explaining .. now it has some issue there ... not sure yet .. but maybe you need to remove the cli.py if the the cli option is None ... |
it should remove the cli.py if there is no CLI selected |
thanks @YurelyCamacho |
🎉 This PR is included in version 0.7.2 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Add example code for the cli option defined by the user: Argparse and Click
Solve #122
-->
How to test these changes
...
Pull Request checklists
This PR is a:
About this PR:
Author's checklist:
Additional information
Reviewer's checklist
Copy and paste this template for your review's note: