Avoid using non-intuitive names ("github", "graphql") as a module name

[JonghoKim-jj]

현재 .graphql 파일들이 src/github/ 디렉토리에 있습니다.
src/ 디렉토리에는 Rust 코드만 있도록 하는게 나을 것 같습니다.
그리고 작업을 하다보니 GitHub GraphQL API 에 요청하는 부분 로직이 점점 복잡해져
src/github/ 안에 Rust 파일들을 추가하고 싶기도 합니다.
프로젝트 루트 디렉토리에 graphql/github/ 을 만들고, .graphql 파일을 전부 옮기면 어떨까요?

graphql-client crate 이용하여 codegen 하는 부분은 아래와 같이 수정하면 됩니다.

Before:

#[derive(GraphQLQuery)]
#[graphql(
    schema_path = "src/github/schema.graphql",
    query_path = "src/github/issues.graphql",
    response_derives = "Debug, Clone"
)]
pub(crate) struct Issues;

After:

#[derive(GraphQLQuery)]
#[graphql(
    schema_path = "graphql/github/schema.graphql",
    query_path = "graphql/github/issues.graphql",
    response_derives = "Debug, Clone"
)]
pub(crate) struct Issues;

[sehkone]

@JonghoKim-jj

GraphQL 스키마 파일 등은 어디에 두어도 상관은 없습니다. 프로젝트의 최상위에 두어도 괜찮고 그게 더 일반적이긴 합니다. 그러나 역시 현재와 같이 src 아래에 두는 경우도 아무런 문제가 없습니다.

이런 경우에는 보통 현재 상태를 유지하는 것이 일반적입니다. 시간을 소모하지 않고, 작업 시 오류 발생 가능성도 제외할 수 있기 때문입니다.

다만, 변경할 때 얻게되는 이득이 확실한 경우에는 변경을 선택해야 합니다. @JonghoKim-jj 께서 주신 설명에서는 확실한 이득이 포함된 것 같지는 않습니다.

한편, 아래 설명은 어떤 의미인지 잘 모르겠습니다. 로직이 복잡해지는 것이 src/github/ 안에 Rust 파일을 추가하고 싶은 이유라는 말씀 같은데, 잘 이해가 가지 않습니다.

그리고 작업을 하다보니 GitHub GraphQL API 에 요청하는 부분 로직이 점점 복잡해져
src/github/ 안에 Rust 파일들을 추가하고 싶기도 합니다.

[JonghoKim-jj]

@sehkone

제가 제안드린 것은 refactoring 으로 생각하시면 됩니다.
(다음부터 개선안 제안할 때는 제안에 더 강한 근거가 필요하다는 점은 숙지하도록 하겠습니다.)

src/github.rs 에 구현된 로직에는 GitHub GraphQL API 에서 정의한 스키마와 저희가 정의한 GraphQL Query 를 graphql-client 크레이트를 이용하여 codegen 하고, 생성된 struct 들을 가져다 쓰는 과정이 있습니다. GitHub 에서 정의한 스키마이기 때문에 생성된 struct 들에 새로운 field 를 넣는 등의 수정은 불가합니다. 저는 이 과정이 외부 크레이트를 가져다 쓰는 것과 비슷하다고 생각하여 src/ 에 정의된 저희 로직과 분리하는 것을 생각했습니다.
GraphQL API 를 먼저 설계하거나 가져오고, 이후 Rust 로 내부 로직을 구현하는 것이 개발과정이니 분리하는게 자연스럽다고 판단했습니다.

그리고 작업을 하다보니 GitHub GraphQL API 에 요청하는 부분 로직이 점점 복잡해져
src/github/ 안에 Rust 파일들을 추가하고 싶기도 합니다.

이 부분에 대해 다시 설명드리자면,
현재 main branch 에 있는 src/github.rs 가 600줄이 넘고, 진행 중인 PR, 진행 예정인 이슈 등을 반영하면 더 길어질 것 같아 파일을 분리하는 것은 어떨까 생각하고 있었습니다.

[sehkone]

@JonghoKim-jj, 무슨 고민을 하고 있는지 이해가 되었습니다. 그런데, 문제점 진단을 달리해야 맞겠습니다.

현재의 문제점은 github 모듈의 하위 모듈이 사용할 디렉터리와 GraphQL 스키마 관련 파일을 넣어두는 디렉터리가 충돌하는 것이네요.

github 모듈을 github.rs 내에서만 처리한다면 모르겠으나, 하위 모듈, 예를 들자면 github::sub_module을 만든다고 하면 src/github/sub_module.rs를 만들어야 하는데, 이 경우에는 src/github 디렉터리와 GraphQL 관련 파일을 넣어두는 디렉터리가 같게 되는 것이지요. 이것은 분명하게 수정해야 할 문제입니다.

이 문제의 원인은 근본적으로 이름 충돌이므로 github 모듈의 이름과 GraphQL 파일을 저장하는 디렉터리 이름 중에 최소한 하나를 수정해야 하겠습니다. 즉, 네이밍 이슈인 것입니다. GitHub과 관련된 것이라는 이유로 가리지 않고 github으로 이름 붙여 버리니 생긴 문제이지요.

제가 Code Summit에서 이야기 한 것처럼 이름을 잘 짓는 것은 아무리 강조해도 지나치지 않습니다.

@JonghoKim-jj 께서는 GraphQL 파일을 저장하는 디렉터리의 위치가 문제인 것처럼 기술하였는데, 그것은 본질은 아닙니다. 충돌하는 이름을 변경하면서 경로도 함께 변경할 수 있는데 이는 부수적인 것입니다.

따라서, 다음 순서대로 생각해 보는 것을 제안합니다.

• github 모듈의 이름은 이대로가 맞는가? 제게는 딱 와 닿지는 않습니다. 별도로 .github과 같은 디렉터리를 사용하고 있기도 하고, GitHub은 고유명사이므로 실제 이 모듈의 역할을 이해하는 데 맞지는 않는 것 같아요. 이 repository의 내용 중에 GitHub과 관련되지 않은 게 없습니다. Repository 이름에 github이 들어간 것으로 충분하고, 다른 상황에서는 안 쓰는 게 바람직합니다.
• GraphQL 스키마 관련 파일을 넣어두는 디렉터리 이름으로서 github은 적당한가? 마찬가지 이유로 적당해 보이지 않습니다.
• GraphQL 관련 파일 디렉터리의 이름을 바꾸기로 정하였다면, 이참에 src 아래에 두지 않고 그 위로 옮기는 것도 고려할 수 있습니다. 이름만 바꾸는 것과 경로까지 바꾸는 것은 거의 대등한 정도의 수고를 들이기 때문이지요.

Cc: @danbi2990 @sophie-cluml

[sehkone]

이 이슈의 제목을 다음처럼 적는다면 문제점을 잘 드러내면서 꼭 해야 할 일로 전달이 됩니다.

• Fix Naming Conflict from Concurrent Use of github
• Avoid Using github as a Directory or Filename

[JonghoKim-jj]

이 리포지토리 기능은 크게 2가지로 볼 수 있습니다.

1. GitHub GraphQL API 에 요청하여 GitHub 관련 정보들을 가져오고, DB 에 저장
2. DB 에 저장된 데이터에 기반하여 GitHub 관련 통계를 GraphQL API 로 제공

그런데 이 기능을 하는 기존 모듈들의 이름이 github, graphql 로 정해져 있는데,
GitHub GraphQL API 나 이 리포지토리가 사용자에게 제공하는 GitHub 관련 통계를 제공하는 GraphQL API 나,
모두 github, graphql 등과 관련이 있기 때문에 모듈의 이름만으로 기능을 추론하기 어렵습니다.

이에 따라 직관적인 naming 을 위해 아래 세 가지를 제안합니다.

• 기존 모듈 github 을 fetch 로 renaming
• 기존 src/github/ 디렉토리에 있던 GraphQL 파일들을 graphql/fetch/ 디렉토리로 이동
• 기존 모듈 graphql 을 dashboard (또는 api) 로 renaming

기존 github 모듈의 기능은 GitHub GraphQL API Endpoint 로부터 필요한 데이터를 요청하고, 결과를 DB 에 저장하는 것입니다. 즉, fetch 해오는 기능이라 볼 수 있습니다.
그러므로 이 모듈 이름을 fetch 로 바꾸면 더 직관적일 것 같습니다.
그리고 경로들을 변경하는 김에 기존 src/github/ 디렉토리에 있던 GraphQL 파일들을 graphql/fetch/ 디렉토리로 옮긴다면 추후 모듈에 submodule 을 생성할 때 같은 디렉토리에 Rust 파일과 GraphQL 파일이 섞이는 것을 막을 수 있습니다.

기존 graphql 모듈의 기능은 DB 에 저장된 데이터들을 이용하여 사용자에게 GitHub 관련 통계를 GraphQL API 형태로 제공하는 것입니다.
그러므로 이 모듈 이름을 dashboard (또는 api) 로 바꾸면 더 직관적이리라 생각합니다.

[danbi2990]

제안 감사합니다.

말씀하신대로 현재 github.rs 파일 내용이 많습니다. 대표적으로 GitHubIssue 같이 저희가 선언한 구조체(+ 앞으로 추가될 Try(From) trait)가 큰 비중을 차지합니다. 이는 현재 database/discussion.rs 파일처럼 database 폴더 아래로 리팩토링할 예정입니다 (관련 Discussion). 그러면 github.rs 파일에는 fetch 관련 함수만 남게 됩니다. send_github_issue_query 함수의 내용도 Try(From) trait이 구현되면 많이 줄어들 것으로 예상합니다.

기존 모듈 github 을 fetch 로 renaming

콘 말씀처럼 github 폴더명은 일단 부적절해 보입니다. GitHubIssue 구조체 등을 database 폴더 아래로 옮기고나면 fetch 기능을 수행하는 함수만 남게 됩니다. 그래서 fetch 폴더명도 일리는 있어 보이는데 제 생각엔 이것도 광범위해 보입니다. github_client.rs 정도로 표현하면 어떨까 싶습니다.

기존 src/github/ 디렉토리에 있던 GraphQL 파일들을 graphql/fetch/ 디렉토리로 이동

GraphQL 파일들이 github/ 폴더에 있는건 수정이 필요해 보이네요. 최근 리팩토링을 진행할 때 github/query 폴더에 넣으려는 시도가 있었는데 당장은 필요없어 보여서 철회했던 적이 있습니다(관련 코멘트의 첫번째 인용 단락).

다만, 변경에 대해서는 GraphQL 파일들을 위한 디렉토리이므로 src/github_client/query/ 가 직관적이지 않나 싶네요. GitHubIssue등을 database 폴더 아래로 모두 옮기고나면 github_client.rs 파일의 내용이 많이 줄어들게 되고 하위 모듈이 필요 없어질 가능성이 커 보입니다. 그러면 src/github_client/ 폴더에 query/ 폴더만 존재하기 때문에 차라리 src/github_query/ 폴더에 GraphQL 파일들을 모아두는게 적절해 보이기도 합니다.

기존 모듈 graphql 을 dashboard (또는 api) 로 renaming

이 내용도 한번 논의된 적이 있긴 합니다. 저희가 graphql을 client, server 둘 다의 용도로 사용하기 때문에 저도 개발하면서 인지 부담을 경험했던 적이 있습니다(관련 코멘트의 두번째 인용 단락). 당시 소피와 논의했었는데 레오도 동일한 불편을 겪었기 때문에 변경에 대해 다시 고려해봐도 좋을 것 같습니다. @sophie-cluml

[sehkone]

이렇게 하도록 해요.

project-root/
└── src/                     # 소스 코드 루트
    ├── api/                 # GraphQL API 구현
    └── outbound/            # GitHub과 통신 구현
        └── graphql/         # .graphql 파일들

[JonghoKim-jj]

@danbi2990 @sophie-cluml 이견 없으시면 제가 self assign 하고 진행해도 될까요?

[danbi2990]

넵 좋습니다.

[danbi2990]

이슈에 Assignee 및 Project 설정 부탁드립니다.

[danbi2990]

todo-list에 Start date 도 입력 부탁드립니다. 노션에서 'Issue Management Roles & Responsibilities' 검색해서 R&R 확인해보시면 좋을 것 같습니다.