Skip to content

engine: Centralized leafnode-related code in one place#1001

Merged
genieCS merged 1 commit into
githru:mainfrom
genieCS:core/leaf-node
Oct 15, 2025
Merged

engine: Centralized leafnode-related code in one place#1001
genieCS merged 1 commit into
githru:mainfrom
genieCS:core/leaf-node

Conversation

@genieCS
Copy link
Copy Markdown
Contributor

@genieCS genieCS commented Oct 14, 2025

Related issue

#1000

Result

Queue constructor를 봤을 때 어떤 기준으로 sort하는지 알 수 있고(latestFirstComparator) comparator 함수를 봤을 때 commit.branches.length가 아닌 isLeafNode라는 이름을 사용함으로써 commit.branches.length가 어떤 케이스인지
이해하기 쉽다.

Work list

Discussion

@genieCS genieCS requested review from a team as code owners October 14, 2025 13:44
@genieCS genieCS self-assigned this Oct 14, 2025
Copy link
Copy Markdown
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.

오! 멋진 리팩토링입니다!!!
Test 까지 너무 좋네용!

Copy link
Copy Markdown
Contributor

@nxnaxx nxnaxx left a comment

Choose a reason for hiding this comment

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

LGTM!! 테스트 코드까지 꼼꼼하게 고생하셨습니다👍🏻


export const isLeafNode = (node: CommitNode): boolean => node.commit.branches.length > 0;

export const latestFirstComparator = (a: CommitNode, b: CommitNode): number => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

함수명이 명확해져서 좋습니다👍🏻

const leafNodes: CommitNode[] = [];
commitDict.forEach((node) => node.commit.branches.length && leafNodes.push(node));
return leafNodes;
return [...commitDict.values()].filter(isLeafNode);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isLeafNode까지 재사용해서 훨씬 깔끔해졌네요ㅎㅎ

@genieCS genieCS merged commit 7b66beb into githru:main Oct 15, 2025
2 checks passed
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