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

Add 'Update Swagger doc' workflow #894

Merged
merged 2 commits into from
Oct 30, 2021

Conversation

computerphilosopher
Copy link
Member

@computerphilosopher computerphilosopher commented Oct 26, 2021

make swagger 를 자동으로 실행해 추가 커밋을 만드는 workflow를 추가하였습니다.

이 action은 core나 rest api쪽 코드에 변화가 있을 때에만 실행됩니다.

    paths:
      - 'src/core/**/**.go'
      - 'src/api/rest/server/**/**.go'

문서 디렉토리의 파일 중 추가된 것이 있으면 자동으로 커밋합니다. 커밋 메세지는 "Update Swagger Docs" 입니다.

      - name: Commit generated Swagger docs
        uses: stefanzweifel/git-auto-commit-action@v4
        with:
          file_pattern: src/api/rest/docs/**
          commit_message: Update Swagger docs

author는 원 커밋 저자로, committer는 'actions-user'로 기록됩니다.

image

@computerphilosopher computerphilosopher changed the title Update 'make swagger' workflow Update 'make swagger' workflow(#804) Oct 26, 2021
@computerphilosopher computerphilosopher changed the title Update 'make swagger' workflow(#804) Update 'make swagger' workflow Oct 26, 2021
@jihoon-seo
Copy link
Member

@computerphilosopher 아주 좋네요!
고생이 많으셨습니다.
기여 감사합니다! 😊

@jihoon-seo jihoon-seo linked an issue Oct 27, 2021 that may be closed by this pull request
@jihoon-seo jihoon-seo changed the title Update 'make swagger' workflow Add 'Update Swagger doc' workflow Oct 27, 2021
@jihoon-seo jihoon-seo added the lgtm This PR is acceptable by at least one reviewer label Oct 27, 2021
@seokho-son
Copy link
Member

@hermitkim1 한번 살펴봐주시겠어요~? ^^

@jihoon-seo
Copy link
Member

생각난 사항을 메모해 봅니다. 😊

main 브랜치에서 'src/core/**/**.go' 또는 'src/api/rest/server/**/**.go' 파일이 변경되어
workflow가 실행되었는데
make swag 실행 결과 Swagger doc은 변경사항 없이 그대로인 경우

  • 아무 commit도 추가되지 않나요?
  • 아니면, 빈 commit이 추가되나요? 😊

      - name: Commit generated Swagger docs
        uses: stefanzweifel/git-auto-commit-action@v4
        with:
          file_pattern: src/api/rest/docs/**
          commit_message: Update Swagger docs

이 부분을 보니
Swagger doc이 변경사항 없이 그대로인 경우
아마도 아무 commit도 추가되지 않을 것 같네요.. 😊

Copy link
Member

@yunkon-kim yunkon-kim left a comment

Choose a reason for hiding this comment

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

@computerphilosopher
반복적인 작업 또는 놓치고 넘어갈 수 있는 부분을 해결해주는 매우 유용한 기여를 해주셨네요 ^^
감사드립니다 💟

몇 가지 코멘트 확인 부탁드려요~~

runs-on: ubuntu-18.04
strategy:
matrix:
go-version: [ '1.17' ]
Copy link
Member

Choose a reason for hiding this comment

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

@seokho-son

혹시, Go version을 1.17으로 버전업 하셨을까요?

README의 실행 및 개발환경에 표기된 추천 Go version은 1.16으로 표기되어 있어 문의드립니다 ^^

Copy link
Member

Choose a reason for hiding this comment

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

@hermitkim1 아직 버전업 하지 않았습니다. 최신 릴리스에서 1단계 정도 낮게 가져가고 있습니다. (그래서 1.16)
저도 해당 워크플로우에서의 1.17을 사용하는 것으로 되어 있어서, 버전을 살짝 고민해보긴 했는데..

꼭 소스 구동 환경과 동일하게 맞출 필요는 없을 수도 있겠다는 생각입니다.. ^^
조만간 소스 구동 환경도 1.17로 올릴 가능성이 높기도 하구요 ^^

Copy link
Member

Choose a reason for hiding this comment

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

@computerphilosopher 기여해주신 1.17 유지해 주시고요 ^^

Comment on lines +10 to +11
update-swagger-doc:
name: Update Swagger doc
Copy link
Member

Choose a reason for hiding this comment

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

기여해주신 워크플로우도 cb-tumblebug 리포지토리에서 동작하도록 제한할 필요성을 느낍니다 ^^

이유는 (본 워크플로우를 모르는) 기여자의 Forked repository에서 혹시라도 워크플로우가 수행되는 경우 Upstream와 forked간에 꼬이는 불상사를 방지하기 위함입니다.

Ref #895

Suggested change
update-swagger-doc:
name: Update Swagger doc
update-swagger-doc:
if: github.repository == 'cloud-barista/cb-tumblebug'
name: Update Swagger doc


- name: Install swag
run: |
go install github.com/swaggo/swag/cmd/swag@latest
Copy link
Member

Choose a reason for hiding this comment

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

버전을 명시하는 것이 좋을 것 같습니다 ^^

Copy link
Member

Choose a reason for hiding this comment

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

저의 생각은..
이 라인에 버전을 명시해 놓으면
아마도 "GitHub Actions autoupdate" 의 자동 업데이트 기능으로는 업데이트가 되지 않을 것 같고
(uses: 필드에 써 있는 것은 자동 업데이트가 될텐데, run: 필드 밑에 써 있어서.. 😊)

예를 들어 이 라인에 v1.7.0 으로 적혀 있는데
기여자가 v1.7.1 을 이용하여 Swagger doc 을 업데이트 하면
이 workflow가 다시 v1.7.0 을 이용하여 Swagger doc 을 업데이트하는 일이 발생할 수도 있을 것 같아서

저는 latest 가 좋지 않을까 생각을 하였으며..
이 사안과 관련하여 @hermitkim1 님과 오프라인에서 이야기를 나누었습니다. 😊

Copy link
Member

Choose a reason for hiding this comment

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

@computerphilosopher 께서 원하시는 방향으로 진행하시면 될 것 같습니다 😄

@computerphilosopher
Copy link
Member Author

computerphilosopher commented Oct 27, 2021

@jihoon-seo

PR전에 다음과 같은 테스트를 진행했습니다.

테스트 결과 api와 무관한 코드를 수정할 경우 아무 커밋도 생기지 않았습니다.

image

@computerphilosopher
Copy link
Member Author

@hermitkim1

forked repo에서 실행을 막아야 할 것 같다는 리뷰 사항 반영해 다시 push 하였습니다.

swagger의 버전을 고정하는 것과 최신 버전으로 하는 것 중에 어느 것이 최선인지는 잘 모르겠네요. 일단 그대로 두었습니다.

@yunkon-kim
Copy link
Member

/lgtm

Copy link
Member

@yunkon-kim yunkon-kim left a comment

Choose a reason for hiding this comment

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

/approve

@seokho-son
Copy link
Member

모두~ 감사합니다!!!
머지하겠습니닷!★★

@seokho-son seokho-son merged commit 165731c into cloud-barista:main Oct 30, 2021
@jihoon-seo jihoon-seo added the hacktoberfest-accepted hacktoberfest-accepted label Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted hacktoberfest-accepted lgtm This PR is acceptable by at least one reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automate opening a PR to update Swagger API doc by a workflow
4 participants