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

Deduplicate code for checking k8scluster enablement #1689

Conversation

sykim-etri
Copy link
Member

assets/cloud.conf에 기재된 k8scluster enablement를 확인하는 코드들의 중복을 제거하고, 클러스터 버전 확인 부분을 업데이트하였습니다.

Copy link
Member

@seokho-son seokho-son left a comment

Choose a reason for hiding this comment

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

@sykim-etri 감사합니다! LGTM.
마이너한 사항에 대한 리뷰 의견을 추가하였습니다.

@@ -1932,6 +1844,69 @@ func convertSpiderNodeGroupStatusToTbK8sNodeGroupStatus(spNodeGroupStatus Spider
return TbK8sNodeGroupInactive
}

func CheckK8sClusterEnablement(connectionName string) error {
Copy link
Member

Choose a reason for hiding this comment

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

대문자로 시작하는 함수이므로, 린트를 위해서 주석 추가 부탁드립니다.
(이왕이면, 나머지 소문자로 시작하는 내부 함수도 가급적 주석을 추가해주시면 좋을 것 같습니다. 현 PR에서 수정할 필요까지는 없을 것 같습니다.)

Copy link
Member Author

Choose a reason for hiding this comment

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

대문자로 시작할 필요가 없는 함수였습니다.
타 내부 함수에 대한 주석을 추가한 PR은 추후 별도로 진행하겠습니다.

@sykim-etri sykim-etri force-pushed the deduplicate-check-k8scluster-enablement branch from 91c3bdc to cc5b095 Compare July 26, 2024 05:11
@sykim-etri sykim-etri force-pushed the deduplicate-check-k8scluster-enablement branch from cc5b095 to 1081fee Compare July 26, 2024 05:18
@seokho-son
Copy link
Member

감사합니다! LGTM.

@seokho-son
Copy link
Member

/approve

@github-actions github-actions bot added the approved This PR is approved and will be merged soon. label Jul 26, 2024
@cb-github-robot cb-github-robot merged commit 87d89c5 into cloud-barista:main Jul 26, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved and will be merged soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants