-
Notifications
You must be signed in to change notification settings - Fork 4
641: wip #665
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?
641: wip #665
Conversation
Available PR Commands
See: https://github.com/tahminator/codebloom/wiki/CI-Commands |
Title641: wip PR TypeEnhancement, Other Description
Diagram Walkthroughflowchart LR
BashScript["Bash script (.sh)"] -- replaced by --> BunTS["Bun TypeScript script (.ts)"]
Workflow["GitHub Action composite"] -- setup Node/pnpm/Bun --> Env["Script runtime env"]
Env -- installs --> Deps["scripts package deps"]
BunTS -- runs --> Maven["Maven fmt/lint/verify"]
|
| Relevant files | |||||
|---|---|---|---|---|---|
| Enhancement |
| ||||
| Miscellaneous |
| ||||
| Configuration changes |
| ||||
| Dependencies |
|
67c5fe1 to
a6ba20b
Compare
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
| await $`./mvnw checkstyle:check`; | ||
|
|
||
| // compile | ||
| await $`./mvnw -B verify -Dmaven.test.skip=true --no-transfer-progress`; |
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.
Suggestion: Use 'with' on '$' to fail early and stream output; also set a working directory to the repo root to avoid path issues. This ensures the Maven command inherits stdio and fails on non-zero exit reliably in Bun contexts. [possible issue, importance: 6]
| await $`./mvnw -B verify -Dmaven.test.skip=true --no-transfer-progress`; | |
| await $.cwd(process.cwd()).with({ stdin: 'inherit', stdout: 'inherit', stderr: 'inherit' })`./mvnw -B verify -Dmaven.test.skip=true --no-transfer-progress`; |
| main() | ||
| .then(() => { | ||
| process.exit(0); | ||
| }) | ||
| .catch((e) => { | ||
| console.error(e); | ||
| process.exit(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.
Suggestion: Avoid explicit process.exit calls; they can terminate before streams flush and mask unhandled rejections. Let the promise chain determine exit status by rethrowing or returning non-zero via shell. [general, importance: 7]
| main() | |
| .then(() => { | |
| process.exit(0); | |
| }) | |
| .catch((e) => { | |
| console.error(e); | |
| process.exit(1); | |
| }); | |
| main().catch((e) => { | |
| console.error(e); | |
| // Non-zero exit by setting process.exitCode; allows stdout/stderr to flush. | |
| process.exitCode = 1; | |
| }); |
| - name: Install deps | ||
| shell: bash | ||
| run: pnpm --dir .github/scripts install --frozen-lockfile | ||
|
|
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.
Suggestion: Pin the working directory using 'working-directory' instead of relying on '--dir' to avoid path resolution differences across runners. This reduces risk of installing in the wrong location. [general, importance: 6]
| - name: Install deps | |
| shell: bash | |
| run: pnpm --dir .github/scripts install --frozen-lockfile | |
| - name: Install deps | |
| shell: bash | |
| working-directory: .github/scripts | |
| run: pnpm install --frozen-lockfile |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
a8a8e6f to
118e014
Compare
83c86aa to
933567d
Compare
|
/ai |
|
/review |
|
/describe |
|
/improve |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Title641: wip PR TypeEnhancement Description
Diagram Walkthroughflowchart LR
Bash["Legacy bash scripts"] -- replaced by --> BunTS["Bun TypeScript scripts"]
BunTS -- used in --> CIBackend["CI: Backend tests"]
BunTS -- used in --> CIFrontend["CI: Frontend tests"]
Colors["ANSI color helpers"] -- improve logs --> BunTS
Workflows["GitHub Actions workflows"] -- setup bun/cache/run --> CIBackend
Workflows -- setup bun/cache/run --> CIFrontend
AIReview["AI review workflow"] -- add Notion context & tuning --> Workflows
|
| Relevant files | |||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Enhancement | 6 files
| ||||||||||||
| Miscellaneous | 5 files
| ||||||||||||
| Configuration changes | |||||||||||||
| Dependencies | 1 files
|
641: wip fix coloring 641: wip fix coloring pt2 641: update ai review 641: add load-secrets 641: add load-secrets pt2 641: add load-secrets pt3 641: add load-secrets pt4 641: add load-secrets pt5 fix writing logic 641: add load-secrets pt5 fix writing logic wdekfjnewkgfn
641
Description of changes
Checklist before review
Screenshots
Dev
Staging