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

Refactor status codes handing logic #408

Merged
merged 1 commit into from
Sep 22, 2024
Merged

Refactor status codes handing logic #408

merged 1 commit into from
Sep 22, 2024

Conversation

hahwul
Copy link
Member

@hahwul hahwul commented Sep 22, 2024

@ksg97031
먼저 좋은 기능 만들어주셔서 감사합니다! #406
dev 브랜치에서 테스트해보는데 --status-codes가 사용되지 않는 경우에도 update_status_codes 가 호출되는 것 같습니다.

코드를 살펴봤는데, exclude_codes가 Bool 타입으로 사용되는 것 같진 않은데 any_to_bool의 인자값으로 쓰이고 있어 이로 인해 해당 부분에서 false가 되고 "" 가 아니기에 무조건 돌아가지 않았을까 싶습니다. 의도된 로직일 수도 있어 일단 조심스럽게 수정 버전으로 PR 드려봅니다.

    # Set status code
    if any_to_bool(@options["status_codes"]) == true || any_to_bool(@options["exclude_codes"]) != ""
      update_status_codes
    end

검토 부탁드려요 :D

@hahwul hahwul self-assigned this Sep 22, 2024
@github-actions github-actions bot added 📑 documentation Improvements or additions to documentation 🔎 detector Issue for Detector 🔬 analyzer Issue for Analyzer 📦 output-builder Issue for output builder (format) 💊 spec Issue for test codes 💌 deliver Issue for Deliver 🦺 github-action Issue for GitHub actions ⚙️ options Issue for options (flag) 🛥️ workflow Issue for Workflows 🥢 mini-lexer Issue for mini-lexer and mini-parser 🏷️ tagger Issue for tagger ⛱️ config Issue for Configuration labels Sep 22, 2024
@hahwul hahwul changed the base branch from main to dev September 22, 2024 06:27
@hahwul hahwul removed 📑 documentation Improvements or additions to documentation 🔎 detector Issue for Detector 🔬 analyzer Issue for Analyzer 📦 output-builder Issue for output builder (format) 💊 spec Issue for test codes 💌 deliver Issue for Deliver 🦺 github-action Issue for GitHub actions ⚙️ options Issue for options (flag) 🛥️ workflow Issue for Workflows 🥢 mini-lexer Issue for mini-lexer and mini-parser 🏷️ tagger Issue for tagger ⛱️ config Issue for Configuration labels Sep 22, 2024
Copy link
Member

@ksg97031 ksg97031 left a comment

Choose a reason for hiding this comment

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

앜 이런실수를.. 감사합니다!!

@ksg97031 ksg97031 merged commit 7d92c9f into dev Sep 22, 2024
9 checks passed
@hahwul hahwul deleted the hahwul-dev branch September 22, 2024 07:18
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