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

[View] cluster 요약에서 cluster의 releaseTag를 보여주는 방식 변경 #763

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rakseong
Copy link
Contributor

@rakseong rakseong commented Oct 6, 2024

Related issue

#727

Result

  • 현재
    cluster에 latestReleaseTag 정보를 가지고 display
image image

  • 수정
    cluster에 모든 ReleaseTag 정보를 가지고 display할 때 latestReleaseTag 판단
image image

Work list

(with @pithesun)

  1. Cluster Type의 latestReleaseTag 삭제, clusterTags 추가
  2. Summary.Util의 function getInitData에서 clusterTags 정보 추가
  3. Summary에서 Summary.Util의 function getCommitLatestTag 호출하여 최신 ReleaseTag display

Discussion

#756 merge 이후 latestReleaseTag 관련 내용 수정 필요

@rakseong rakseong requested review from a team as code owners October 6, 2024 17:27
Copy link
Contributor

@ytaek ytaek left a comment

Choose a reason for hiding this comment

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

LGGTM!!!

@@ -19,7 +19,7 @@ export type Summary = {
export type Cluster = {
clusterId: number;
summary: Summary;
latestReleaseTag: string;
clusterTags: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -36,7 +36,7 @@ function tagToNumber(tag: string, maxLength: number): number {
* @param tags
* @returns
*/
function getCommitLatestTag(tags: string[]): string {
export function getCommitLatestTag(tags: string[]): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

오! 간단한 test case를 만들어보기 좋은 public 함수가 하나 나왔군요!!!

Copy link
Contributor

@xxxjinn xxxjinn left a comment

Choose a reason for hiding this comment

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

LGTM!!!!!!!✨✨❤️

@ytaek
Copy link
Contributor

ytaek commented Nov 9, 2024

확인되셨으면 머지 부탁드리겠습니다!

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