-
Notifications
You must be signed in to change notification settings - Fork 10
feat: normalize changelog #400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Summary of ChangesHello @noahchoii, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request overhauls the project's changelog generation system to produce more structured and informative release notes. By moving from generic semantic versioning categories to component-specific groupings, and by enriching entries with PR and contributor details, the changelogs become significantly more user-friendly. The change also streamlines dependency update logging and removes redundant changelogs for application-level packages, ensuring that the generated history is both comprehensive and concise. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
이 PR은 changeset의 체인지로그 생성 방식을 개선하여, 변경 사항을 컴포넌트/모듈 스코프 기준으로 그룹화하도록 수정합니다. 이를 위해 scripts/changelog/index.js와 scripts/changelog/post-process.js 두 개의 새로운 스크립트가 추가되었습니다. 전반적인 로직과 접근 방식은 좋으나, 몇 가지 개선점을 제안합니다. index.js에서는 사용되지 않는 변수를 제거하고, post-process.js에서는 스타일 가이드에 맞게 함수 네이밍을 수정하고, 복잡하고 잠재적인 서식 오류를 유발할 수 있는 들여쓰기 로직을 단순화할 것을 제안합니다.
| const commitBuffer = () => { | ||
| if (currentBuffer.length === 0) return; | ||
|
|
||
| // 첫 번째 라인이 비어있는지 확인 (헤더만 있는 경우) | ||
| const isFirstLineEmpty = currentBuffer[0].trim() === ''; | ||
|
|
||
| const formattedBuffer = currentBuffer.map((line, index) => { | ||
| // 제목 라인은 그대로 유지 | ||
| if (index === 0) return line; | ||
|
|
||
| // 개행 라인은 그대로 유지 | ||
| if (line.trim() === '') return line; | ||
|
|
||
| // 첫 번째 라인이 비어있으면 들여쓰기 하지 않음 (형제 항목들) | ||
| if (isFirstLineEmpty) return line; | ||
|
|
||
| // 첫 번째 라인이 있으면 들여쓰기 함 (하위 항목들) | ||
| return '\t' + line; | ||
| }); | ||
|
|
||
| const currentContent = formattedBuffer.join('\n').trim(); | ||
| entries.push({ scope: currentScope, content: currentContent }); | ||
|
|
||
| currentBuffer = []; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commitBuffer 함수의 들여쓰기 로직이 복잡하고, index.js에서 이미 적용된 들여쓰기와 충돌하여 최종 마크다운 파일의 형식을 깨뜨릴 수 있습니다. 예를 들어, index.js에서 2칸 스페이스로 들여쓰기된 내용에 탭(\t)이 추가되어 일관성 없는 들여쓰기가 발생합니다. index.js에서 이미 포맷팅된 내용을 그대로 사용하도록 commitBuffer 함수를 단순화하는 것이 좋습니다. 또한, 스타일 가이드 3.3절(152라인)에 따르면 들여쓰기는 스페이스 4칸을 사용해야 하지만, 현재 코드에서는 탭 문자를 사용하고 있습니다.
const commitBuffer = () => {
if (currentBuffer.length === 0) return;
const currentContent = currentBuffer.join('\n').trim();
if (currentContent) {
entries.push({ scope: currentScope, content: currentContent });
}
currentBuffer = [];
};References
- 스타일 가이드 3.3절에 따라 들여쓰기는 스페이스 4칸을 사용해야 합니다. (link)
scripts/changelog/index.js
Outdated
| const commits = await Promise.all( | ||
| changesets.map(async (cs) => { | ||
| if (cs.commit) { | ||
| let { links } = await getInfo({ | ||
| repo: options.repo, | ||
| commit: cs.commit, | ||
| }); | ||
| return links.commit; | ||
| } | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scripts/changelog/post-process.js
Outdated
| function TitleCase(str) { | ||
| return str | ||
| .toLowerCase() | ||
| .replace(/\b\w/g, (char) => char.toUpperCase()) | ||
| .replace(/-/g, ' '); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
스타일 가이드 3.2. 네이밍 컨벤션(144라인)에 따르면 함수명은 camelCase를 사용해야 합니다. TitleCase 함수명을 titleCase로 변경하고, 이 함수를 호출하는 286, 347라인도 함께 수정하는 것이 좋겠습니다. 파일 내 다른 함수들은 이 규칙을 잘 따르고 있습니다.
| function TitleCase(str) { | |
| return str | |
| .toLowerCase() | |
| .replace(/\b\w/g, (char) => char.toUpperCase()) | |
| .replace(/-/g, ' '); | |
| } | |
| function titleCase(str) { | |
| return str | |
| .toLowerCase() | |
| .replace(/\b\w/g, (char) => char.toUpperCase()) | |
| .replace(/-/g, ' '); | |
| } |
References
- 스타일 가이드 3.2절에 따라 함수명은 camelCase를 사용해야 합니다. (예: userName, calculatePrice) (link)
.changeset/config.json
Outdated
| "snapshot": { | ||
| "prereleaseTemplate": "datetime" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason this snapshot version update was added here?? Previously, it seems the test version was actually incorrectly labeled as a pre-release. If this was added as a test version, how about adding a test version script to package.json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I think I accidentally uploaded this while trying to figure out how to package test during the PR stage for my prerelease! I'll delete this!
.github/workflows/release.yml
Outdated
| name: "Release Packages" | ||
|
|
||
| on: | ||
| push: | ||
| paths: | ||
| - '.changeset/**' | ||
| - 'packages/**' | ||
| - ".changeset/**" | ||
| - "packages/**" | ||
| branches: | ||
| - main | ||
|
|
||
| concurrency: ${{ github.workflow }}-${{ github.ref }} | ||
|
|
||
| jobs: | ||
| release: | ||
| name: Release | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout branch | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version-file: package.json | ||
| registry-url: 'https://registry.npmjs.org/' | ||
| always-auth: true | ||
|
|
||
| - uses: pnpm/action-setup@v4 | ||
|
|
||
| - name: Install Dependencies | ||
| run: | | ||
| pnpm install --frozen-lockfile | ||
| - name: Install | ||
| uses: ./.github/composite/install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like the prettier file isn't running here. Please check it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format command only works within the package directory, so it can't format files in the root path. I'll manually run the command and add it for now!
| const replacedChangelog = changeset.summary | ||
| .replace(/^\s*(?:pr|pull|pull\s+request):\s*#?(\d+)/im, (_, pr) => { | ||
| let num = Number(pr); | ||
| if (!isNaN(num)) prFromSummary = num; | ||
| return ''; | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, I see that the PR number has been added to the changeset. Is the changeset document something the developer writes manually? In my experience, the PR number doesn't get added automatically to that changeset, so it should always be empty. Do we need to add it now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR number, scope, commit, and all other details added here are automatically included when the changeset version command runs. Therefore, users do not need to add any content separately!
| if (response.ok) { | ||
| const { title } = await response.json(); | ||
| // "feat(Scope): description" 패턴 추출 | ||
| const scopeMatch = title.match(/^\w+\(([^)]+)\):/); | ||
| if (scopeMatch) { | ||
| scopeFromPR = scopeMatch[1]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[NIT]
Handling simple typos will likely be necessary later on.! The approach that comes to mind right now is fuzzy string search. It seems best to implement this if issues arise down the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are definitely situations where having typos corrected is convenient! However, fuzzy matching algorithms are useful for searching for similar words together during searches, and it seems difficult to determine whether a word judged as a typo is actually a typo or not. So in this case, I also think the most stable approach would be for the maintainer to directly check the PR title to ensure it's entered correctly without typos!
It might be good to discuss this part further and brainstorm ideas later!
Description of Changes
apps/since they don't require version control.How It Works
scripts/changelog/index.js
post-process.jscan generate the actual changelog.[SCOPE:xxx]because it needs to be merged with other entries of the same scope during post-processing.scripts/changelog/post-process.js
getFilesToProcess).postProcessChangelog).## 1.0.0-beta.10).groupedEntriesin the format<scope:content>otherEntries.groupedEntries, writing out the content for each scope based on the scope criteria.groupedEntriesis complete, I addotherEntries, which contains changes not belonging to any scope.Screenshots
Checklist
Before submitting the PR, please make sure you have checked all of the following items.