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

Setup flake8 CI job #207

Merged
merged 1 commit into from
May 28, 2023
Merged

Conversation

NoharaMasato
Copy link
Contributor

@NoharaMasato NoharaMasato commented Apr 10, 2023

Summary:

  • Fix flake8 errors
$ flake8 .
geojson/codec.py:30:80: E501 line too long (82 > 79 characters)
geojson/codec.py:32:80: E501 line too long (88 > 79 characters)
geojson/geometry.py:15:80: E501 line too long (82 > 79 characters)
geojson/utils.py:212:11: E275 missing whitespace after keyword
geojson/utils.py:214:13: E275 missing whitespace after keyword
geojson/utils.py:216:13: E275 missing whitespace after keyword
  • Setup CI job for flake8

@codecov
Copy link

codecov bot commented Apr 10, 2023

Codecov Report

Merging #207 (c35a19e) into main (3a3a1b3) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #207   +/-   ##
=======================================
  Coverage   98.25%   98.25%           
=======================================
  Files          19       19           
  Lines         801      801           
=======================================
  Hits          787      787           
  Misses         14       14           
Impacted Files Coverage Δ
geojson/utils.py 95.53% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@NoharaMasato
Copy link
Contributor Author

I don't know why this change decreased the test coverage and caused a job failure on CI. Please let me know if I need to add unit tests for the utils.py

@NoharaMasato
Copy link
Contributor Author

Added a test for utils.py in #210

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Looks good, a couple of suggestions.

.github/workflows/lint.yml Outdated Show resolved Hide resolved
.github/workflows/lint.yml Outdated Show resolved Hide resolved
@hugovk
Copy link
Member

hugovk commented May 27, 2023

Or a better idea, let's do it using the existing pre-commit!

Add this to .pre-commit-config.yaml:

 repos:
+  - repo: https://github.com/PyCQA/flake8
+    rev: 6.0.0
+    hooks:
+      - id: flake8
+
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.4.0
     hooks:

And replace lint.yml with:

name: Lint

on: [push, pull_request, workflow_dispatch]

permissions:
  contents: read

jobs:
  lint:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-python@v4
        with:
          python-version: "3.x"
      - uses: pre-commit/action@v3.0.0

rayrrr
rayrrr previously approved these changes May 27, 2023
@rayrrr rayrrr self-requested a review May 27, 2023 23:42
@rayrrr rayrrr dismissed their stale review May 27, 2023 23:42

did not see full conversation in thread

@NoharaMasato NoharaMasato force-pushed the setup-flake8 branch 2 times, most recently from dcacaf1 to 006a74d Compare May 28, 2023 01:37
@NoharaMasato
Copy link
Contributor Author

@hugovk , @rayrrr
Thanks for checking the PR. Updated to use pre-commit hook to run link on CI.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thank you!

@NoharaMasato NoharaMasato merged commit 3a4a274 into jazzband:main May 28, 2023
10 checks passed
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Dec 9, 2023
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

3 participants