-
Notifications
You must be signed in to change notification settings - Fork 0
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(ci): make Python version dynamic #68
Conversation
Reviewer's Guide by SourceryThis pull request makes the python version in the vulnerability workflow dynamic by using the cookiecutter variable. Flow diagram for dynamic Python version in vulnerability workflowgraph TD
A[Cookiecutter Template] -->|python_version variable| B[vulnerability.yaml]
B -->|Used in| C[Poetry Setup Action]
C -->|Configures| D[Python Version for Vulnerability Checks]
style A fill:#f9f,stroke:#333,stroke-width:2px
style B fill:#bbf,stroke:#333,stroke-width:2px
style C fill:#dfd,stroke:#333,stroke-width:2px
style D fill:#fdd,stroke:#333,stroke-width:2px
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @Nidhi091999 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Thanks a lot, @Nidhi091999! LGTM, but I would like @JaeAeich to have a look as well before merging.
Please do the same in actions.yaml as well for Note the raw tag, you will have to bring that below otherwise |
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.
Please see the comment.
Done |
{{ cookiecutter.project_name_dashed }}/.github/actions/setup/poetry/action.yaml
Show resolved
Hide resolved
One way to test if your changes are good and not breaking anything is to:
Note: There is a possibility that you some of the hooks when you run |
We will implement CI for all of this stuff maybe you can take the initiative if you want, happy to help. Wasn't a priority for us, but I can see the need now. @Nidhi091999 maybe open an issue for this with title like |
5ea6f84
to
c412f21
Compare
Thank you for the commands. I have tested my changes, and there are no liniting issues. However, there are some linting warnings in other parts of the code, which I will address in a separate PR shortly. |
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.
LGTM!! Nice :)
@Nidhi091999 seems like your commits aren't verified, maybe go ahead and create a gpg key, add it to github and so that all your other commits are verified. look into these instruction. |
Thanks for the instructions! I've generated a new GPG key and added it to GitHub. |
@uniqueg these changes are good, maybe lets bypass the verified signature rule for now, given @Nidhi091999's further changes will be verfied 🚀 |
Resolves: #67
Summary by Sourcery
CI:
cookiecutter.python_version
in the vulnerability check workflow.