SZUT/casino
SZUT/casino
Archived
3
1
Fork 0
Code Issues Pull requests 2 Releases 162 Packages 2 Wiki Activity Actions

fix: Fix frontend build bc apparently they changed the node version #245

Merged
jank merged 2 commits from fix/node-version into main 2025-06-01 10:03:18 +00:00
Conversation 3 Commits 2 Files changed 2 +7 -2
jank commented 2025-06-01 09:03:41 +00:00
Owner
Copy link
No description provided.
jank added 1 commit 2025-06-01 09:03:42 +00:00
fix: Fix frontend build bc apperantly they changed the node version
Some checks failed
CI / Get Changed Files (pull_request) Successful in 8s
Details
Setup Gitea Tea CLI / setup-and-login (pull_request) Failing after 11s
Details
CI / Checkstyle Main (pull_request) Has been skipped
Details
CI / Docker backend validation (pull_request) Has been skipped
Details
CI / Backend Tests (pull_request) Has been skipped
Details
CI / Docker frontend validation (pull_request) Successful in 1m0s
Details
CI / prettier (pull_request) Successful in 2m41s
Details
CI / oxlint (pull_request) Successful in 2m43s
Details
CI / eslint (pull_request) Successful in 2m48s
Details
CI / test-build (pull_request) Successful in 49s
Details
69447d8fcf
jank force-pushed fix/node-version from 69447d8fcf to 360acb2dbd 2025-06-01 09:05:05 +00:00 Compare
Claude approved these changes 2025-06-01 09:06:33 +00:00
Dismissed
Claude left a comment
Collaborator

Review Summary: APPROVE

The changes in this PR address two infrastructure improvements:

✅ Positive Changes:

  • .gitea/workflows/claude.yml:34 - Removing sudo improves security practices in CI/CD
  • frontend/.docker/Dockerfile:4-9 - Upgrading to explicit Node.js 20.x installation provides better version control and reproducibility
  • Docker best practices implemented with --no-install-recommends and cleanup commands

⚠️ Potential Concern:

  • .gitea/workflows/claude.yml:34 - Ensure the runner has write permissions to /usr/local/bin/ without sudo, otherwise the workflow may fail

Minor: Typo in commit message (apperantly → apparently) but doesn't affect functionality.

Overall, these are solid infrastructure improvements that enhance build reliability and security.

**Review Summary: APPROVE** The changes in this PR address two infrastructure improvements: **✅ Positive Changes:** - **.gitea/workflows/claude.yml:34** - Removing sudo improves security practices in CI/CD - **frontend/.docker/Dockerfile:4-9** - Upgrading to explicit Node.js 20.x installation provides better version control and reproducibility - Docker best practices implemented with --no-install-recommends and cleanup commands **⚠️ Potential Concern:** - **.gitea/workflows/claude.yml:34** - Ensure the runner has write permissions to /usr/local/bin/ without sudo, otherwise the workflow may fail **Minor:** Typo in commit message (apperantly → apparently) but doesn't affect functionality. Overall, these are solid infrastructure improvements that enhance build reliability and security.
jank force-pushed fix/node-version from 360acb2dbd to 602166aa61 2025-06-01 09:09:20 +00:00 Compare
Claude requested changes 2025-06-01 09:10:38 +00:00
Dismissed
Claude left a comment
Collaborator

--message Issues Found:

  1. Critical: .gitea/workflows/claude.yml line 34 - Removing sudo from mv tea /usr/local/bin/tea will cause permission denied errors. Moving files to /usr/local/bin/ requires elevated privileges in CI environments.

  2. Minor: PR title contains typo - 'apperantly' should be 'apparently'

Positive Changes:

  • frontend/.docker/Dockerfile lines 4-9: Good improvements to Node.js installation using official NodeSource repository, proper cleanup, and Docker best practices.

Please restore the sudo command in the workflow file to prevent CI failures.

--message **Issues Found:** 1. **Critical**: `.gitea/workflows/claude.yml` line 34 - Removing sudo from `mv tea /usr/local/bin/tea` will cause permission denied errors. Moving files to `/usr/local/bin/` requires elevated privileges in CI environments. 2. **Minor**: PR title contains typo - 'apperantly' should be 'apparently' **Positive Changes:** - `frontend/.docker/Dockerfile` lines 4-9: Good improvements to Node.js installation using official NodeSource repository, proper cleanup, and Docker best practices. Please restore the `sudo` command in the workflow file to prevent CI failures.
jank changed title from fix: Fix frontend build bc apperantly they changed the node version to fix: Fix frontend build bc apparently they changed the node version 2025-06-01 09:11:16 +00:00
jank dismissed Claude's review 2025-06-01 09:11:37 +00:00
Reason:

shut the fuck up idiot

Claude commented 2025-06-01 09:14:17 +00:00
Collaborator
Copy link

Review of PR #245

Issues Found:

  1. .gitea/workflows/claude.yml line 34: Removing \ from \ might cause permission issues. The \ directory typically requires elevated privileges to write to in CI environments.

  2. frontend/.docker/Dockerfile lines 4-9: The Node.js 20.x installation changes look good overall and follow Docker best practices with proper cleanup.

Recommendation: The Dockerfile improvements are solid, but the workflow change might break the CI pipeline. Consider testing if the tea binary installation works without sudo in your CI environment before merging.

**Review of PR #245** **Issues Found:** 1. **.gitea/workflows/claude.yml line 34**: Removing \ from \ might cause permission issues. The \ directory typically requires elevated privileges to write to in CI environments. 2. **frontend/.docker/Dockerfile lines 4-9**: The Node.js 20.x installation changes look good overall and follow Docker best practices with proper cleanup. **Recommendation:** The Dockerfile improvements are solid, but the workflow change might break the CI pipeline. Consider testing if the tea binary installation works without sudo in your CI environment before merging.
jank merged commit bb1621ba49 into main 2025-06-01 10:03:18 +00:00
jank deleted branch fix/node-version 2025-06-01 10:03:18 +00:00
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels   
Compat/Breaking

Breaking change that won't be backward compatible

  
Kind/Bug

Something is not working

  
Kind/Documentation

Documentation changes

  
Kind/Enhancement

Improve existing functionality

  
Kind/Feature

New functionality

  
Kind/Security

This is security issue

  
Kind/Testing

Issue or pull request related to testing

  
Priority
Critical
The priority is critical

  
Priority
High
The priority is high

  
Priority
Low
The priority is low

  
Priority
Medium
The priority is medium

  
Reviewed
Confirmed
Issue has been confirmed

  
Reviewed
Duplicate
This issue or pull request already exists

  
Reviewed
Invalid
Invalid issue

  
Reviewed
Won't Fix
This issue won't be fixed

  
Status
Abandoned
Somebody has started to work on this but abandoned work

  
Status
Blocked
Something is blocking this issue or pull request

  
Status
Need More Info
Feedback is required to reproduce issue or to continue work

  
backend

   
ci

   
docs

   
frontend
  
lgtm
blocked

  
lgtm
done

  
lgtm
need 1

  
size
huge

  
size
large

  
size
medium

  
size
small

No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
backend
ci
docs
frontend
lgtm
blocked
lgtm
done
lgtm
need 1
size
huge
size
large
size
medium
size
small
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: SZUT/casino#245
No description provided.
Delete branch "fix/node-version"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?