githru / githru-vscode-ext

Lightweight but robust Githru for VSCode Extension
Apache License 2.0
50 stars 83 forks source link

[engine] git.worker.ts에 새로운 git format 적용 #765

Closed BeA-Pro closed 1 month ago

BeA-Pro commented 1 month ago

Related issue

Result

git.worker.ts에 #753에서 적용하신 새로운 git format을 적용하였습니다.

Work list

Discussion

@yoouyeon 님이 작성하신 코드를 보고 그대로 적용했는데 잘 동작하는 것 같습니다. 제 예상에는 getGitLog에서 진행 되었던 테스트가 fetchGitLogInParallel에서도 진행되어야 할 것 같은데 한 번 검토 부탁드립니다.

BeA-Pro commented 1 month ago

제가 생각했던 방법도 arg에 제가 지정했던 포맷을 전달하는 것 뿐이라서 이렇게 하면 잘 동작할 것 같습니다! 👏👏👏

이제 getGitLog 함수는 fetchGitLogInParallel 함수로 대체되었기 때문에 어디서도 사용되지 않는거죠?? 개인적으로는 사용하지 않는 함수가 코드에 남아있으면 나중에 혼란스러울수도 있어서 제거하는 것이 좋을 것 같습니다..!

제가 알기로는 @novice1993님이 올려주신 getGitLog 테스트코드가 fetchGitLogInParallel 함수의 동작에 맞춰져 있어서 아마도 getGitLog 테스트코드쪽도 fetchGitLogInParallel를 사용하도록 수정이 필요할 것 같아요.

getGitLog는 이제 사용하지 않는 것은 맞는데, 제가 조금 우려되는 부분은 저의 코드가 테스트 케이스를 돌려본적이 없으니까 만약을 위해 일단은 남겨 놓는 것이 좋을 듯 싶습니다. 나중에 fetchGitLogInParallel의 검증이 어느 정도 완료가 되면 멘토님과 상의하여 삭제하는게 좋을 듯 합니다!

ytaek commented 1 month ago

제가 생각했던 방법도 arg에 제가 지정했던 포맷을 전달하는 것 뿐이라서 이렇게 하면 잘 동작할 것 같습니다! 👏👏👏 이제 getGitLog 함수는 fetchGitLogInParallel 함수로 대체되었기 때문에 어디서도 사용되지 않는거죠?? 개인적으로는 사용하지 않는 함수가 코드에 남아있으면 나중에 혼란스러울수도 있어서 제거하는 것이 좋을 것 같습니다..! 제가 알기로는 @novice1993님이 올려주신 getGitLog 테스트코드가 fetchGitLogInParallel 함수의 동작에 맞춰져 있어서 아마도 getGitLog 테스트코드쪽도 fetchGitLogInParallel를 사용하도록 수정이 필요할 것 같아요.

getGitLog는 이제 사용하지 않는 것은 맞는데, 제가 조금 우려되는 부분은 저의 코드가 테스트 케이스를 돌려본적이 없으니까 만약을 위해 일단은 남겨 놓는 것이 좋을 듯 싶습니다. 나중에 fetchGitLogInParallel의 검증이 어느 정도 완료가 되면 멘토님과 상의하여 삭제하는게 좋을 듯 합니다!

TC 만들고 삭제하면 될 것 같습니다 : )