Skip to content

Show bad configured CI#55

Merged
manuzhang merged 4 commits intomanuzhang:mainfrom
kolibri91:fix/ci
Apr 10, 2023
Merged

Show bad configured CI#55
manuzhang merged 4 commits intomanuzhang:mainfrom
kolibri91:fix/ci

Conversation

@kolibri91
Copy link
Copy Markdown
Contributor

@kolibri91 kolibri91 commented Apr 7, 2023

@manuzhang

Your CI is green even if it contains styling-issues:
e.g. https://github.com/manuzhang/mkdocs-htmlproofer-plugin/actions/runs/4637384298/jobs/8206224207?pr=55
image

There are two issues:

  • You should specify folder names instead of .
  • --exit-zero is most likely not want you want

I'm going to solve all issues with my next PR. I noticed it after my implementation, so it would be easier to adress it there.

@kolibri91 kolibri91 changed the title List files explicitly Show bad configured CI Apr 7, 2023
flake8 htmlproofer/ tests/ setup.py --count --select=E9,F63,F7,F82 --show-source --statistics
# 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
flake8 htmlproofer/ tests/ setup.py --count --max-complexity=10 --max-line-length=127 --statistics
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

the comment above needs update as well

@manuzhang
Copy link
Copy Markdown
Owner

Please help fix style issues after removing --exit-zero

@kolibri91
Copy link
Copy Markdown
Contributor Author

kolibri91 commented Apr 8, 2023

Please help fix style issues after removing --exit-zero

Really? It's done in #57. So, why to double the work?

@manuzhang
Copy link
Copy Markdown
Owner

manuzhang commented Apr 9, 2023

I prefer one PR/commit handling one issue. Combing multiple things in one commit make it hard to check history, revert, etc. You may have two commits in #57 with one handling style, the other external URL.

@kolibri91
Copy link
Copy Markdown
Contributor Author

Fair point, I would do it in the same way if it's really matters. But the project is rather small and the history is easy to overview.

Nevertheless, I will rework the commits

@kolibri91 kolibri91 marked this pull request as ready for review April 9, 2023 17:52
@kolibri91 kolibri91 requested a review from manuzhang April 9, 2023 17:52
@kolibri91
Copy link
Copy Markdown
Contributor Author

Styling issues are fixed by 5b230b8 & ed1dd7f

@manuzhang manuzhang merged commit c012617 into manuzhang:main Apr 10, 2023
@kolibri91 kolibri91 deleted the fix/ci branch April 13, 2023 07:49
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.

2 participants