chore: Remove old Anhang and add some code to the docs #317

Merged
jank merged 2 commits from docs/code into main 2025-06-12 15:33:54 +00:00
Owner
No description provided.
jank added 1 commit 2025-06-12 13:30:13 +00:00
chore: Remove old Anhang and add some code to the docs
All checks were successful
Build docs / build-docs (pull_request) Successful in 29s
Label PRs based on size / Check PR size (pull_request) Successful in 27s
CI / Get Changed Files (pull_request) Successful in 34s
CI / Backend Tests (pull_request) Has been skipped
CI / eslint (pull_request) Has been skipped
Pull Request Labeler / labeler (pull_request_target) Successful in 4s
CI / Checkstyle Main (pull_request) Has been skipped
CI / oxlint (pull_request) Has been skipped
CI / Docker frontend validation (pull_request) Has been skipped
CI / prettier (pull_request) Has been skipped
CI / Docker backend validation (pull_request) Has been skipped
CI / test-build (pull_request) Has been skipped
CI / Playwright (pull_request) Has been skipped
Claude PR Review / claude-code (pull_request) Successful in 1m41s
0e9e729fdf
forgejo-actions added the
size
large
label 2025-06-12 13:30:45 +00:00

👮‍♀️⚠️ This is a friendly reminder that the diff size of this PR is bigger than 200 lines we aim for. Please consider splitting this PR into more digestible pieces!

👮‍♀️⚠️ This is a friendly reminder that the diff size of this PR is bigger than 200 lines we aim for. Please consider splitting this PR into more digestible pieces!
forgejo-actions added the
docs
label 2025-06-12 13:30:50 +00:00
Collaborator

Code Review Feedback

Issues Found:

projektdokumentation/Listings/CoinflipComponent.ts:

  • Line 7 in Anhang.tex: Incorrect language specification for TypeScript file - should be or
  • Lines 140-143: Inconsistent property access pattern with both and suggests API response uncertainty
  • Missing proper null checks in method

projektdokumentation/Listings/CoinflipController.java:

  • Line 14: Unused import should be removed
  • Line 31: Missing user existence validation before game logic

projektdokumentation/Listings/UserEntity.java:

  • Lines 89-90: method uses fragile string replacement that could break if username/email contain replacement patterns

projektdokumentation/Listings/application.properties:

  • Security concern: Hardcoded sensitive values (Stripe keys, JWT secrets) shouldn't appear in documentation examples
  • Configuration inconsistency between development and production values

projektdokumentation/Abkuerzungen.tex:

  • Line 38: Removed API abbreviation may still be referenced elsewhere in document

projektdokumentation/Inhalt/Coinflip.tex:

  • Line 42: Sentence with multiple consecutive references reduces readability

Positive aspects:

  • Good modernization of documentation content
  • Proper demonstration of multi-layer architecture
  • Relevant code examples for the casino platform

Please address the language specification and security concerns before merging.

## Code Review Feedback ### Issues Found: **projektdokumentation/Listings/CoinflipComponent.ts:** - Line 7 in Anhang.tex: Incorrect language specification for TypeScript file - should be or - Lines 140-143: Inconsistent property access pattern with both and suggests API response uncertainty - Missing proper null checks in method **projektdokumentation/Listings/CoinflipController.java:** - Line 14: Unused import should be removed - Line 31: Missing user existence validation before game logic **projektdokumentation/Listings/UserEntity.java:** - Lines 89-90: method uses fragile string replacement that could break if username/email contain replacement patterns **projektdokumentation/Listings/application.properties:** - Security concern: Hardcoded sensitive values (Stripe keys, JWT secrets) shouldn't appear in documentation examples - Configuration inconsistency between development and production values **projektdokumentation/Abkuerzungen.tex:** - Line 38: Removed API abbreviation may still be referenced elsewhere in document **projektdokumentation/Inhalt/Coinflip.tex:** - Line 42: Sentence with multiple consecutive references reduces readability ### Positive aspects: - Good modernization of documentation content - Proper demonstration of multi-layer architecture - Relevant code examples for the casino platform Please address the language specification and security concerns before merging.
jank added 1 commit 2025-06-12 13:36:25 +00:00
Merge main
All checks were successful
CI / Get Changed Files (pull_request) Successful in 9s
CI / eslint (pull_request) Has been skipped
Pull Request Labeler / labeler (pull_request_target) Successful in 4s
CI / Backend Tests (pull_request) Has been skipped
CI / oxlint (pull_request) Has been skipped
CI / Checkstyle Main (pull_request) Has been skipped
CI / prettier (pull_request) Has been skipped
CI / Docker frontend validation (pull_request) Has been skipped
CI / test-build (pull_request) Has been skipped
CI / Docker backend validation (pull_request) Has been skipped
Label PRs based on size / Check PR size (pull_request) Successful in 16s
CI / Playwright (pull_request) Has been skipped
Build docs / build-docs (pull_request) Successful in 26s
Claude PR Review / claude-code (pull_request) Successful in 1m21s
d916fe4942
Collaborator

Code Review

The documentation update looks good overall - nice work removing outdated content and adding relevant code examples. However, there are a few minor issues to address:

Issues:

  1. projektdokumentation/Listings/CoinflipComponent.ts:18+ - Mixed language comments (German comments with English code) should be consistent
  2. projektdokumentation/Listings/CoinflipComponent.ts:247 - Missing newline at end of file
  3. projektdokumentation/Listings/CoinflipComponent.ts:142 - The frontend workaround for backend API inconsistency suggests the backend should be fixed instead
  4. projektdokumentation/Listings/application.properties:53 - Missing newline at end of file
  5. projektdokumentation/Listings/application.properties:11,34,46 - Consider using placeholder values for API keys in documentation instead of real test keys
  6. projektdokumentation/Anhang.tex:23 - Language attribute should be 'typescript' or 'javascript' instead of 'C' for the TypeScript listing

Positive aspects:

  • Great job removing outdated content
  • Comprehensive architectural examples
  • Good cross-referencing between sections
  • Clear documentation structure

The changes effectively improve the documentation quality. Please address the minor issues above.

## Code Review The documentation update looks good overall - nice work removing outdated content and adding relevant code examples. However, there are a few minor issues to address: **Issues:** 1. **projektdokumentation/Listings/CoinflipComponent.ts:18+** - Mixed language comments (German comments with English code) should be consistent 2. **projektdokumentation/Listings/CoinflipComponent.ts:247** - Missing newline at end of file 3. **projektdokumentation/Listings/CoinflipComponent.ts:142** - The frontend workaround for backend API inconsistency suggests the backend should be fixed instead 4. **projektdokumentation/Listings/application.properties:53** - Missing newline at end of file 5. **projektdokumentation/Listings/application.properties:11,34,46** - Consider using placeholder values for API keys in documentation instead of real test keys 6. **projektdokumentation/Anhang.tex:23** - Language attribute should be 'typescript' or 'javascript' instead of 'C' for the TypeScript listing **Positive aspects:** - Great job removing outdated content - Comprehensive architectural examples - Good cross-referencing between sections - Clear documentation structure The changes effectively improve the documentation quality. Please address the minor issues above.
csimonis approved these changes 2025-06-12 13:39:15 +00:00
jank merged commit 7221422d3b into main 2025-06-12 15:33:54 +00:00
jank deleted branch docs/code 2025-06-12 15:33:55 +00:00
Commenting is not possible because the repository is archived.
No reviewers
No milestone
No project
No assignees
4 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#317
No description provided.