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

Update Github Action #10

Merged
merged 9 commits into from
Jul 31, 2021
Merged

Update Github Action #10

merged 9 commits into from
Jul 31, 2021

Conversation

comnamu18
Copy link
Owner

코드 스타일 가이드 관련 깃헙 액션 추가

원래 목적은 unused import 문 black이나 isort로 없애는 거 까지 하려고 하였으나

import를 정적으로 검사하였을 때 생길 수 있는 버그가 있다고 하여 스킵

black이 해주는 기본적인 LINT + 라인 MAX 넘 79

참고링크 : https://ljvmiranda921.github.io/notebook/2018/06/21/precommits-using-black-and-flake8/

@comnamu18
Copy link
Owner Author

comnamu18 commented Jul 30, 2021

마지막 커밋이후 나오는 동작대로면 ok

해당 action 동작 이해 + 확인 되었으면 코멘트 달아주세요

PR CLOSE후, master에 바로 수정된 yaml파일만 올릴 예정입니다.

왜 와이?commit - afdcede 에서 linter가 바꾼 부분 때문에 #9 랑 conflict 날 예정이라

@WraithKim
Copy link
Collaborator

WraithKim commented Jul 31, 2021

근데 F401이랑 F403 무시해도 지금 flake8이 F405에 대해서 오류를 뿜는데, 이러면 unused import에 대해서 깔끔하게 처리하는게 아님.
그리고 위에 올린 사이드이펙트도 봤는데, 저게 동작하는게 신기하긴 하지만 저러면 flake8에서 어차피 F405로 잡혀서 걸러질거 같은데 그러면 결국 문제가 생길 거 같지 않음.
그래서 내 의견은 그냥 F401이랑 F403 무시하지말고 import 관련 오류는 전부 lint로 잡아냈으면 함.

@comnamu18
Copy link
Owner Author

comnamu18 commented Jul 31, 2021

@WraithKim

TL;DR : F시리즈 checker만 있고 linter 없으므로 관련 에러 발생 시 코드 리뷰 하는 형식으로 해서 수동으로 fix하기를 제안

F401, F403 ignore 없애기는 ㅇㅋ 근데 import관련이슈들을 linter가 잡는 건 안될 듯?

  1. import를 정적검사기에서 검사하는 것은 코드 장애를 유발할 수 있다.
  2. 1의 이유로 black에선 import 문에 대한 검사를 feature로 지원하지 않는다.
  3. 현재 사용중인 깃헙 액션 레포에서 python linter관련은 4가지를 제공하는데, 그 중 에러 코드 F시리즈pyflakes에서 정의된 내용인데, 해당 내용들은 보통 Check만 해주지 자동으로 lint까지 지원하는 linter 툴은 없는 듯 함..(현재 사용중인 깃헙 액션에 없는 isort또한 F시리즈에 대한 체크나 린팅은 안 되는 걸로 확인 됨)

@comnamu18 comnamu18 merged commit 91fcd23 into main Jul 31, 2021
@comnamu18 comnamu18 deleted the hotfix/githubaction branch July 31, 2021 13:54
comnamu18 added a commit that referenced this pull request Jul 31, 2021
comnamu18 added a commit that referenced this pull request Jul 31, 2021
comnamu18 added a commit that referenced this pull request Jul 31, 2021
WraithKim pushed a commit that referenced this pull request Aug 3, 2021
* Update main.yml
* Change Base Github Action template
* Fix(Lint) automatically
* Do not ignore F401, F403 in flake8
reference: https://ljvmiranda921.github.io/notebook/2018/06/21/precommits-using-black-and-flake8/
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.

3 participants