refactor(engine): commit classify based type inclueded #416
Conversation
blan19
changed the title
| for (let index = 0; index < CommitMessageTypeList.length; index++) { | ||
| if (lowerCaseMessage.includes(CommitMessageTypeList[index])) { | ||
| type = CommitMessageTypeList[index]; | ||
| } | ||
| } |
There was a problem hiding this comment.
includes를 사용하셨는데 indexOf를 사용하시면 어떨까요?? ㅎㅎ 그냥 아이디어 한 번 던져봤습니다!
There was a problem hiding this comment.
indexOf도 좋은 아이디어 인 것 같아요 ㅎㅎ bb
성능이 막 차이 나지 않는 경우 코드가 좀 더 이뻐보이는 includes를 사용하는 편인데, indexOf로 저기 if문 내부에서 이쁘게 작성하는 팁이 있으실까요? 😀
한번 더 형변환하거나 그러면 과해보이는 것 같은 느낌을 줘서..
// 이 경우 값이 존재하지 않을 때 -1를 뱉고 조건문 내부에도 진입하게 됨
if (lowerCaseMessage.indexOf(CommitMessageTypeList[index])) {
type = CommitMessageTypeList[index];
}There was a problem hiding this comment.
코드가 예쁘게 작성될지는 모르겠는데 indexOf를 사용하면 Discussion에 말씀하신 문제를 해결하실 수 있을 것 같아서 제안드려봤습니다!!
if (lowerCaseMessage.indexOf(CommitMessageTypeList[index]) >= 0) {
if (type === "") type = CommitMessageTypeList[index];
else {
// indexOf(type) 과 indexOf(CommitMessageTypeList[index]) 를 비교하여 더 작은 값을 type으로
}
}코드가 예쁘진 않은데 커밋 타입이 앞에 나온다는 점을 이용해봤습니다.
| * : -> type과 message 구분 | ||
| */ | ||
| const separatorIdx = messagePrefix.search(/[(!:]/); | ||
| for (let index = 0; index < CommitMessageTypeList.length; index++) { |
There was a problem hiding this comment.
forEach를 안쓰고 index를 쓰신 이유가 있을까용?
There was a problem hiding this comment.
처음에 저도 forEach를 쓰는 방안을 생각했었는데, 이 부분은 제가 Discussion에 나와 있는 부분을 고려하여 중간에 break를 걸어 줄 필요가 있다고 생각해서 코드를 작성했었습니다.
for (let index = 0; index < CommitMessageTypeList.length; index++) {
if(lowerCaseMessage.includes(CommitMessageTypeList[index])) {
type = CommitMessageTypeList[index];
...
break;
}
}커밋 타입이 아직은 많지 않고 메세지도 대부분이 길지는 않겠지만
이렇게 이후에 쓸 데 없는 순회를 더 하지 않으려고 초기에 코드를 작성했었어요 ㅋㅋ
이후에 최종 커밋을 하고 PR을 보낼 땐 테스크 코드를 통과하기 위해 break를 지운 후 커밋했습니다.
어떻게 하면 불필요한 순회를 줄일 수 있을 까하여 Discussion에 적어봤습니다!
아 맞습니다!! 이것도 추후에 추가되면 좋을 것 같습니다. githru paper에서 인용한 on the nature of commits 이라는 paper는 아래와 같은 분류를 사용합니다. 이걸 활용해봐도 좋을 것 같네요 👍 @blan19 님, 이것도 이슈로 등록해주시면 좋겠습니다~. |
Related issue
#187
Result
기존에
:prefix로 분류하던 것을 커밋 메세지안에 특정 커밋 타입이 포함되어 있는지 아닌지로 분류하였습니다.Work list
commit.util.ts
CommitMessageType.ts
parser.spec.ts
Discussion
특정 커밋 타입이 포함되어 있는지 아닌지로 분류하기 때문에 CommitMessageType 배열 순서에 의존하고 있습니다.
예를들어 "feat(engine): build stem & CSM based on passed branch name (#198)" 이 커밋 메세지가 존재하고
저희의 커밋 메세지 타입 배열 순서가 아래와 같다면
커밋 타입이
feat이여야 하는게 배열 순서에 의존하게 되어build로 분류되어 나오게 됩니다.