-
-
Notifications
You must be signed in to change notification settings - Fork 76
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 codecov, pipenv, black, pre-commit #83
Conversation
Codecov Report
@@ Coverage Diff @@
## master #83 +/- ##
=========================================
Coverage ? 85.04%
=========================================
Files ? 4
Lines ? 341
Branches ? 81
=========================================
Hits ? 290
Misses ? 27
Partials ? 24 Continue to review full report at Codecov.
|
for more information, see https://pre-commit.ci
Overall great initiative there's only one thing in hesitant about, which is pipenv. Pipenv is mainly geared towards application development, I’d much rather use poetry or similar which is meant for library/package development. Shouldn’t be too hard to migrate. What do you think? |
fair point! :-) |
6857138
to
0c375b5
Compare
I also update the readme and bumped the version in the project.toml to 0.1.26 I am also thinking about adding bandit at some point. |
Bandit is nice, pyflakes would also be nice. |
yes - but not in this pr :-) |
python -m pip install --upgrade pip | ||
pip install poetry==1.1.11 | ||
poetry install | ||
- name: Test with pytest |
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.
Shouldn’t we enforce all our static checks here as well? Like black formatting.
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.
the are enforced with pre-commit. this also runs on every commit here as check
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.
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.
Every thing we’ve got in pre-commit should be enforced in GH-Actions as well.
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.
Agreed, I was just brainstorming :)
On Mon, 11 Oct 2021 at 20:27, Martin Eigenmann ***@***.***> wrote:
I also update the readme and bumped the version in the project.toml to
0.1.26
I am also thinking about adding bandit at some point.
Bandit is nice, pyflakes would also be nice.
yes - but not in this pr :-)
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#83 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAULT2VZ462MVUM74BZBWV3UGMT75ANCNFSM5FXOY5PQ>
.
--
Med vänlig hälsning
Alexander Hultnér
|
Is pre commit being run in CI?
On Mon, 11 Oct 2021 at 20:30, Martin Eigenmann ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In .github/workflows/python-test.yml
<#83 (comment)>:
> - # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
- flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
- - name: Test with pytest
- run: |
- pytest
+ - uses: ***@***.***
+ - name: Set up Python 3.9
+ uses: ***@***.***
+ with:
+ python-version: 3.9
+ - name: Install dependencies
+ run: |
+ python -m pip install --upgrade pip
+ pip install poetry==1.1.11
+ poetry install
+ - name: Test with pytest
the are enforced with pre-commit. this also runs on every commit here as
check
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#83 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAULT2WMMQHV76QPOBBLFVTUGMUL7ANCNFSM5FXOY5PQ>
.
--
Med vänlig hälsning
Alexander Hultnér
|
yes :-) It runs check locally only for files in the current commit and for everything in the cloud. |
Ohh, I’m on my phone making dinner, totally missed that!
On Mon, 11 Oct 2021 at 20:32, Martin Eigenmann ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In .github/workflows/python-test.yml
<#83 (comment)>:
> - # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
- flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
- - name: Test with pytest
- run: |
- pytest
+ - uses: ***@***.***
+ - name: Set up Python 3.9
+ uses: ***@***.***
+ with:
+ python-version: 3.9
+ - name: Install dependencies
+ run: |
+ python -m pip install --upgrade pip
+ pip install poetry==1.1.11
+ poetry install
+ - name: Test with pytest
But it is enforced as an action - it runs as check.
[image: image]
<https://user-images.githubusercontent.com/2293142/136837739-97897bca-49a7-486d-acfd-cb379d3ba918.png>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#83 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAULT2S7OM2S6OL2LG5AWFLUGMUVNANCNFSM5FXOY5PQ>
.
--
Med vänlig hälsning
Alexander Hultnér
|
@Hultner do you think this pr is ok like it is? |
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.
Looks good to me 👍
This follows what was done at jazzband/icalevents#83.
This follows what was done at jazzband/icalevents#83.
Add codecov, pipenv, black, pre-commit
This follows what was done at jazzband/icalevents#83.
This follows what was done at jazzband/icalevents#83.
I have added the following things:
Pre-commit was added by someone else and it makes sense I think.
Black allows us to remove any guesswork about formatting 😄
Makes repeated builds much more reliable.