Skip to content

[vscode] Azure DevOps URL 파싱 지원 및 getRepo 함수 개선#801

Merged
SingTheCode merged 3 commits into
mainfrom
bugfix/#798_azure-devops-url-parsing-error
Jul 25, 2025
Merged

[vscode] Azure DevOps URL 파싱 지원 및 getRepo 함수 개선#801
SingTheCode merged 3 commits into
mainfrom
bugfix/#798_azure-devops-url-parsing-error

Conversation

@SingTheCode

@SingTheCode SingTheCode commented Jul 22, 2025

Copy link
Copy Markdown
Contributor

Related issue

Closes #798

Result

Azure DevOps URL 파싱 케이스 추가 및 getRepo 함수를 개선하였습니다.

Work list

🔧 기능 개선: Azure DevOps URL 파싱 지원

기존 문제: getRepo 함수가 GitHub URL만 지원하여 Azure DevOps 저장소에서 오류 발생
해결 방안: Azure DevOps URL 패턴을 지원하도록 정규식 패턴 추가
지원 URL 형식:
https://organization@dev.azure.com/organization/project/_git/repository-name
https://dev.azure.com/organization/project/_git/repository-name

🧪 테스트 코드 추가

새 파일: src/test/utils/getRepo.spec.ts
테스트 범위:
GitHub URL 다양한 형식 검증 (HTTPS/SSH, .git 확장자 유무, 사용자명 포함 등)
Azure DevOps URL 형식 검증
잘못된 URL 형식에 대한 에러 처리 검증

Discussion

Azure DevOps 사용자들의 요청에 따라 URL 파싱 지원을 추가했습니다. 기존 GitHub URL 파싱 기능은 그대로 유지하면서 Azure DevOps URL 패턴을 추가로 지원하도록 구현했습니다.

특히 다음 부분들을 중점적으로 검토해주시면 좋겠습니다:

  1. Azure DevOps URL 정규식 패턴이 올바른지
  2. 에러 메시지가 사용자에게 충분히 명확한지
  3. 테스트 케이스가 모든 시나리오를 커버하는지

ytaek and others added 2 commits July 22, 2025 09:24
- Azure DevOps URL 형식 지원 추가
- GitHub URL 파싱 로직 개선
- 더 명확한 에러 메시지 제공
- getRepo 함수에 대한 포괄적인 테스트 케이스 추가
@SingTheCode SingTheCode self-assigned this Jul 22, 2025
@SingTheCode
SingTheCode requested a review from a team as a code owner July 22, 2025 14:34
@SingTheCode SingTheCode added bug Something isn't working 💻 vscode fix and removed bug Something isn't working labels Jul 22, 2025
@SingTheCode SingTheCode changed the title fix(vscode): Azure DevOps URL 파싱 지원 및 getRepo 함수 개선 [vscode] Azure DevOps URL 파싱 지원 및 getRepo 함수 개선 Jul 22, 2025
@ytaek

ytaek commented Jul 23, 2025

Copy link
Copy Markdown
Contributor

상세한 pr description 감사합니다!
브랜치를 main으로 바로 올려주시면 리뷰 시작하겠습니다!
(타겟 브랜치 수정이 안되면 닫고 새로 올려주셔도 됩니다!)

@SingTheCode
SingTheCode changed the base branch from boot to main July 23, 2025 03:04

@ytaek ytaek left a comment

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.

코드는 완전 LGTM입니다!!
헌데 boot branch 쪽 수정사항이 같이 들어가서.. ㅜ.ㅜ
해당 내용 빼서 다시 올려주시면 바로 approve 하겠습니다!

@@ -0,0 +1,121 @@
import { getRepo } from "../../utils/git.util";

describe("getRepo", () => {

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.

오우.. 감동입니다 😭 첫 PR부터 Test case 가 포함되어 있네요!!!

Comment on lines +6 to +14
const url = "https://github.com/owner/repo.git";
const result = getRepo(url);

expect(result).toEqual({
owner: "owner",
repo: "repo",
});
});

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.

(사소) Test case에서도 동일한 logic들은 (stateless하다면) 밖으로 빼도 괜찮을 것 같습니다 👍
혹시나 수정하실 예정이면 다음 PR에 해주셔도 좋을 것 같습니다!

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.

아, URL(input)만 딱 다르고, 다른 부분이 모두 동일하다면 TC의 입력값을 다르게 하는
parameterized test로 바꾸는 것도 괜찮을 것 같습니다!

https://23life.tistory.com/entry/jest-testeach%EB%A5%BC-%ED%86%B5%ED%95%B4-%EC%97%AC%EB%9F%AC-%EC%9E%85%EB%A0%A5%EA%B0%92-%ED%85%8C%EC%8A%A4%ED%8A%B8-%EA%B0%84%EA%B2%B0%ED%95%98%EA%B2%8C-%EA%B5%AC%ED%98%84%ED%95%98%EA%B8%B0

해당 건 이슈로 등록해주셔도 좋을 것 같습니다!
(다른 tc들도 이런 것 있는지 찾아보는 것도 좋을 것 같습니다)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

의견 너무 감사드립니다!!!

it.each 사용하려고 했는데, 현재 vscode 프로젝트에서 테스트 라이브러리로 mocha, jest 두개가 사용되고 있어 *.spec.ts 가 mocha 로 타입추론이 되네요ㅠㅠ

vscode extention 에서는 mocha를 공식적으로 권장하고 있는데 저희 프로젝트에서는 mocha가 설정만 되어있고 샘플 테스트 코드만 구현되어있는 상태라 어느쪽으로 통일해야할지 고민이 됩니다!

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.

그러고보니 vscode 쪽은 거의 tc 가 없긴하네요.
getGitLog.spec.ts 하나 밖에 없으니

*.spec.ts를 mocha 안쓰고 jest로 변경해주셔도 괜찮습니다!!!
다음 PR에서 진행하시지요 : )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

넵! mocha 걷어내는 작업 따로 Issue 파서 진행하도록 하겠습니다ㅎㅎ

Comment on lines +75 to +86
it("Azure DevOps URL should parse correctly", () => {
const url = "https://organization@dev.azure.com/organization/organization-sub-name/_git/repository-name";
const result = getRepo(url);

expect(result).toEqual({
owner: "organization",
repo: "repository-name",
});
});

it("Azure DevOps URL without username should parse correctly", () => {
const url = "https://dev.azure.com/organization/organization-sub-name/_git/repository-name";

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.

저희는 사내에서 github.sec.**.net을 쓰고 있습니다 😸

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

github에 .net으로 끝나는 도메인도 있는지는 몰랐네요ㅎㅎ 정책이 정해진다면 해당 케이스도 추가하도록 하겠습니다!

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.

samsung.net 입니다 👿

Comment on lines +96 to +101
describe("Invalid Git URL formats", () => {
it("should throw error for invalid URL format", () => {
const invalidUrl = "invalid-url-format";

expect(() => getRepo(invalidUrl)).toThrow(
'Invalid Git remote config format: "invalid-url-format". Expected format: [https?://|git@]github.com/owner/repo[.git] or https://organization@dev.azure.com/organization/project/_git/repository-name'

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.

Negative case 까지!! 완벽하네요!

throw new Error("git remote config should be: [https?://|git@]${domain}/${owner}/${repo}.git");
const gitHubPattern =
/(?:https?|git)(?::\/\/(?:\w+@)?|@)(?:github\.com)(?:\/|:)(?:(?<owner>[^/]+?)\/(?<repo>[^/]+?))(?:\.git|\/)?$/m;
const azureDevOpsPattern =

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.

(사소, 추후에..) azure 말고 다른 형태의 url이 들어오면 어떻게 대응하면 좋을까요?
모든 케이스에 대해서 이렇게 추가할 수는 없을 것 같긴 하네요!

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.

정책적으로도 좀 얘기해봐도 좋을 것 같긴 합니다 : )
알려진(known source) public repo만 한다던지... 하는 정책을 세울 수도 있으니!

@SingTheCode SingTheCode Jul 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

domain, owner, repo가 remote url에 할당되어있는 방법이 저마다 달라서 말씀주신대로 public repo에 한해서만 대응하면 좋을 것 같습니다!

정책적으로 정해진다면 사용자에게 다음 액션을 유도하는 것도 사용성을 높일 수 있는 방법이라고 생각됩니다ㅎㅎ (OO, OO repo만 지원됩니다. 또는 현재는 지원하지 않는 형식의 레포지토리입니다. 메일로 현재 레포의 주소를 알려주세요 (메일 전송 버튼 클릭)

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.

domain, owner, repo가 remote url에 할당되어있는 방법이 저마다 달라서 말씀주신대로 public repo에 한해서만 대응하면 좋을 것 같습니다!

정책적으로 정해진다면 사용자에게 다음 액션을 유도하는 것도 사용성을 높일 수 있는 방법이라고 생각됩니다ㅎㅎ (OO, OO repo만 지원됩니다. 또는 현재는 지원하지 않는 형식의 레포지토리입니다. 메일로 현재 레포의 주소를 알려주세요 (메일 전송 버튼 클릭)

넵 이 내용에 대해서는 정책이나 지식 정도로 지정해서, 다른 contributor들도 알 수 있게 하면 좋을 것 같습니다.
조금 더 다른 분들과 토론 해봐도 좋을 것 같아서
위 내용 그대로 새로 이슈로 만들어주시고 discussion 라벨 달아주시면 어떨까요?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

이슈 생성해서 공유하겠습니다! 감사합니다ㅎㅎ

/https:\/\/(?:\w+@)?dev\.azure\.com\/(?<owner>[^/]+?)\/(?<project>[^/]+?)\/_git\/(?<repo>[^/]+?)(?:\/)?$/m;
const patterns = [gitHubPattern, azureDevOpsPattern];

let gitRemote: { owner: string; repo: string } | null = null;

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.

(궁금) null 을 굳이 넣으신 이유는 무엇일까용?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

gitRemote 의 타입을 { owner: string; repo: string } 로만 설정하고 초기값을 설정하지 않거나 { owner: '', repo: '' }으로 설정할 수 있지만, gitRemote가 정상적인 플로우로 값이 할당되지 않았을 때 비어있다라는 것을 명시하는 것이 목적입니다!

에러 발생 조건으로 !owner.length || !repo.length 를 넣을 수도 있지만 !gitRemote가 더 명시적인 코드라고 생각했습니다ㅎㅎ

위 내용은 의견 주시는대로 변경해도 괜찮습니다ㅎㅎ

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.

오. 넵. 그렇군요
(궁금) 혹시 이 경우에 flow의 에러로 undefined가 아닌 null이 할당되는 경우가 있을까요?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

gitRemote 의 변수에 값을 할당하지 않는(undefined) 상황을 가정해서 말씀하시는게 맞을까요??
제가 봤을 때는 없어보입니다! 혹시 그럴 수 있는 상황이 있으면 알려주시면 감사하겠습니다ㅎㅎ

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.

gitRemote 의 변수에 값을 할당하지 않는(undefined) 상황을 가정해서 말씀하시는게 맞을까요?? 제가 봤을 때는 없어보입니다! 혹시 그럴 수 있는 상황이 있으면 알려주시면 감사하겠습니다ㅎㅎ

아 특별히 null과 undefined를 엄격하게 나누는 기준이 있을까 해서 여쭤봤습니다 : )
스타일들이 다 다르니까요 ㅎㅎ

@Zy0ung

Zy0ung commented Jul 23, 2025

Copy link
Copy Markdown
Member

Java의 JUnit 스타일만 보다가 BDD 스타일 테스트는 처음 봤는데,
각 테스트 케이스 설명이 명확하게 드러나서 읽기 정말 편하네요 :)
덕분에 테스트 의도를 빠르게 파악할 수 있었어요! ✨

This reverts commit d8270d1.
@SingTheCode

SingTheCode commented Jul 24, 2025

Copy link
Copy Markdown
Contributor Author

Java의 JUnit 스타일만 보다가 BDD 스타일 테스트는 처음 봤는데, 각 테스트 케이스 설명이 명확하게 드러나서 읽기 정말 편하네요 :) 덕분에 테스트 의도를 빠르게 파악할 수 있었어요! ✨

@Zy0ung
도움이 되어서 다행입니다ㅎㅎ 리뷰 감사합니다!

@SingTheCode

Copy link
Copy Markdown
Contributor Author

코드는 완전 LGTM입니다!! 헌데 boot branch 쪽 수정사항이 같이 들어가서.. ㅜ.ㅜ 해당 내용 빼서 다시 올려주시면 바로 approve 하겠습니다!

@ytaek
boot 초기 설정 커밋 revert 했습니다! 꼼꼼히 확인해주셔서 감사합니다ㅎㅎ

@SingTheCode
SingTheCode requested a review from ytaek July 24, 2025 08:24

@Jxxunnn Jxxunnn left a comment

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.

LGTM 고생하셨습니다! 👍

@SingTheCode

Copy link
Copy Markdown
Contributor Author

LGTM 고생하셨습니다! 👍

@Jxxunnn 리뷰 감사합니다ㅎㅎㅎㅎ

@ytaek ytaek left a comment

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.

LGreatTM!!!!!!!!!!!!!!!!!!!!!!

@SingTheCode
SingTheCode merged commit 45471f5 into main Jul 25, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[fix]: Git 원격 저장소(Azure DevOps) URL 파싱 실패로 인한 에러 발생

4 participants