electron-builder 오픈 소스에 인생 첫 PR 을 날려보다! - ②

지난 글

https://gukjan9.tistory.com/148

electron-builder 오픈 소스에 인생 첫 PR 을 날려보다! - ①

 

지난 글에 이어서 쓰도록 하겠다.

 

PR 날리는 방법

PR 거리를 찾았기 때문에 그 다음은 어떻게 오픈 소스에 PR 을 날리는지 알아보았다.

1. 기여할 레포지토리를 fork 해온다.
2. fork 후 브랜치를 새로 생성하여 작업을 한다.
3. commit 후 push 한다.
4. push 후 내 github 에 fork 해온 레포지토리를 확인해보면 Compare & Open pull request 가 활성화 된다.
5. 프로젝트마다 설정해둔 컨벤션에 맞게 pr 내용을 작성 후 Merge 요청을 한다.

 

다른 사람들의 브랜치 이름들을 참고하면서 내 브랜치 이름을 fix/update-appimage 로 생성하였다.

 

PR 날리기

나름대로 코드를 수정하고 Commit 후 Merge 를 하면

내 Github 에 fork 해온 repository 에 Compare & pull request 버튼이 활성화 되고 클릭하면 원본 repository 인 electron-builder 에 pr 을 날릴 수가 있다.

 

 

이렇게 pr 을 생성하고 나면

electron-builder 에 pull request 탭에 내 게시글이 올라가고 repository 에 설정해둔 각종 봇들이 실행되면서 충돌나는거 없는지 등등의 검사를 실행한다.

 

 

그리고 내 github 메인 페이지에 contribution activity 에도 해당 내역이 올라와있다.

 

여기까지 하면 이제 repository 관리자가 내 pr 을 읽고 피드백을 남겨줄 때 까지 기다리는 일만 남았다.

 

PR 이후

https://github.com/electron-userland/electron-builder/pull/8539

fix(linux): continue update process even if AppImage is not available by gukjan9 · Pull Request #8539 · electron-userland/elec

 

하루 정도 기다리니 관리자로 추정되는 한 사람으로부터 피드백이 들어왔고 물어보는 것에 대해 상세히 설명하며 말을 주고 받았다.

 

// doInstall 함수의 일부

protected doInstall(options: InstallOptions): boolean {
    const appImageFile = process.env["APPIMAGE"]!
    if (appImageFile == null) {
      throw newError("APPIMAGE env is not defined", "ERR_UPDATER_OLD_FILE_NOT_FOUND")
    }
    
    // 원래는 아래 try-catch문 없이 unlinkSync(appImageFile) 만 있었음

    // https://stackoverflow.com/a/1712051/1910191
    try {
      unlinkSync(appImageFile)
    } catch(e: any) {
      this._logger.warn("Previous version of AppImage is not available");
    }
    
    						.
                            			.
                            			.
}

 

(대충 요약)

관리자 : 일단 unlinkSync 가 실행되려면 위에 AppImageFile 을 먼저 체크하지 않니? 없으면 밑에까지 실행이 안될텐데

나 : AppImageFile 은 AppImage 의 경로를 환경변수로 정의를 해줬냐는 의미다. 해주지 않았을 경우 에러가 발생한다. (레퍼런스 첨부)

관리자 : usecase 를 자세하게 설명해줘라. 파일이 존재하지 않는다면 empty/stub 파일을 먼저 경로에 생성을 해두는 것이 어떤가? (첨부한 레퍼런스에 대해 어쩌고 저쩌고 설명)

나 : (첨부한 레퍼런스는 그저 AppImageFile 경로를 정의해주지 않았을 때 에러가 발생한다는 것을 참고하기 위한 링크라고 설명)

관리자 : 타당한 pr 이라고 생각하진 않지만 아직 너의 use case 를 설명하지 않았으니 해줘라. 다운로드 폴더에서 AppImage 를 실행하면 다운로드 폴더의 해당 파일이 업데이트 되지 않는가? 아니면 어떤 사용자가 지정한 경로에서도 작동되게끔 하고 싶은건가?

나 : custom path 는 따로 지정해주지 않으면 에러를 내뱉기 때문에 지정해주었을 뿐이다. 예를 들어 코드에서 AppImageFile 경로를 /Downloads 폴더로 지정해두면 보통의 사용자의 경우 문제가 없이 업데이트가 실행되지만 만약에 사용자가 기본 다운로드 경로를 다르게 지정해두고 실행한다면 AppImage 가 /Downloads 폴더에 없기 때문에 unlinkSync 가 실행 중단이 되고 업데이트가 실패하게 된다. 그래서 try-catch 문을 사용해 해당 경로에 파일이 없을 때 unlinkSync 를 건너뛰고 그 다음 프로세스를 진행하면 이 다음 업데이트부터는 정상적으로 돌아갈 것 이다.

관리자 : 근데 이렇게 하면 무한 루프를 돌 것 같다.

나 : 무한 루프는 안 돌고... (경우에 따른 자세한 프로세스 설명)

관리자 : 뭔가 이번 케이스는 특수한 경우인 것 같다. 해결 방법으로 AppImage 경로에 파일이 존재하지 않는다면 mv 로 /Documents 로 직접 옮겨주고 경로도 /Documents 로 리셋을 해라. 그 후 unlink → quitAndInstall or restart app → app.quit() 해줘라.

나 : 알았다. Thank you~

 

로 대충 이야기가 마무리 된 것 같다.

 

내가 코딩을 하는 입장에선 앱을 빌드할 때 이미 내가 지정한 AppImage 경로가 존재하는데 빌드한 앱을 사용할 실제 사용자 입장에선 도대체 이 경로를 무슨 수로 알고 AppImage 파일을 해당 경로에 옮겨두겠냐는 점에서 의문이 들었었다.

 

unlinkSync 가 결국엔 기존의 AppImage 를 삭제하는 역할인데 해당 경로에 기존 AppImage 가 없으면 unlinkSync 가 실패로 끝나고 해당 함수가 실행 중단이 되어 결국엔 업데이트가 진행되지 않는 점이 조금 어색해보였던건데

 

하지만 나는 그저 한 사람의 개발자이고 이 repository 를 관리하는 사람 입장에선 내 상황이 쉽게 일어나지 않는 상황이고 그렇다 해서 굳이? 같은 느낌으로 반려 시킨 것 같다. (try-catch 로 바꾼다해서 크게 문제될 건 없을 것 같긴 하지만...)

 

또 그들 입장에선 라이브러리에 반영 시킬 정도의 pr 은 아니라고 판단하였지만 그래도 내 의견을 성심성의껏 검토하고 내 상황에 맞게 몇 가지 해결 방법을 제시해주는 것을 보고 '아 이런 것이 개발 문화인가' 싶은 생각도 조금 들었다. 꽉 막혀있지 않고 열린 마인드로 이렇게 해보는건 어때? 하는 느낌

 

이렇게 내 첫 pr 은 끝이 났다.

끝내 반영은 되지 않아 살짝 아쉽긴 하지만 개발자로서 복붙만 하는 개발자가 아닌 그 안으로 들어가 라이브러리의 사용성에 대해 고민을 해보고 더 나은 방향에 대해 생각해볼 기회를 짧게나마 가져본 것 같아 매우 만족스럽다.