[fix] 수업 상세 정보의 점수가 비었을 경우 음수 반환 #156
Conversation
WalkthroughThis pull request introduces version 0.8.1 of the project, focusing on several key modifications across multiple files. The changes primarily involve updating version numbers, refining error handling, and adjusting semester key mappings. The modifications span GitHub Actions workflows, Rust package configurations, and application logic, with a particular emphasis on improving robustness and consistency in handling semester-related data and error scenarios. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant CourseGrades as CourseGradesApplication
participant Model as ClassScore Model
App->>CourseGrades: Request semester grades
CourseGrades->>CourseGrades: Convert semester key
CourseGrades->>Model: Parse grade scores
Model-->>CourseGrades: Handle empty/missing scores
CourseGrades-->>App: Return grade information
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (15)
💤 Files with no reviewable changes (2)
🧰 Additional context used🪛 actionlint (1.7.4).github/workflows/release-ios.yml24-24: shellcheck reported issue in this script: SC2086:info:1:64: Double quote to prevent globbing and word splitting (shellcheck) 27-27: shellcheck reported issue in this script: SC2086:info:7:18: Double quote to prevent globbing and word splitting (shellcheck) 93-93: shellcheck reported issue in this script: SC2086:info:1:10: Double quote to prevent globbing and word splitting (shellcheck) .github/workflows/release.yml25-25: shellcheck reported issue in this script: SC2086:info:1:64: Double quote to prevent globbing and word splitting (shellcheck) 🔇 Additional comments (19)packages/rusaint/tests/application/course_grades.rs (1)
The added null check and early return pattern prevents potential panics when no class with detail is found. This change aligns well with the PR's objective of handling empty/missing data gracefully. packages/rusaint/src/application/chapel/mod.rs (1)
The winter semester key has been changed from "0923" to "093". While this change appears to be part of a broader standardization across modules, we should verify that this doesn't affect existing winter semester data processing. ✅ Verification successfulSemester key change is consistently implemented The change from "0923" to "093" for winter semester is consistently implemented across all relevant modules:
All modules follow the same standardized pattern:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any hardcoded references to the old winter semester key
# Search for any remaining references to the old winter semester key
rg -l "0923"
# Search for usage patterns of the new winter semester key
rg -l "093"
Length of output: 305 Script: #!/bin/bash
# Check the context of "093" usage in all relevant files to verify consistent semester key usage
echo "=== Checking chapel/mod.rs ==="
rg "093" -C 2 packages/rusaint/src/application/chapel/mod.rs
echo -e "\n=== Checking personal_course_schedule/mod.rs ==="
rg "093" -C 2 packages/rusaint/src/application/personal_course_schedule/mod.rs
echo -e "\n=== Checking lecture_assessment/mod.rs ==="
rg "093" -C 2 packages/rusaint/src/application/lecture_assessment/mod.rs
echo -e "\n=== Checking course_grades/mod.rs ==="
rg "093" -C 2 packages/rusaint/src/application/course_grades/mod.rs
echo -e "\n=== Checking course_schedule/mod.rs ==="
rg "093" -C 2 packages/rusaint/src/application/course_schedule/mod.rs
Length of output: 1531 packages/rusaint/src/application/lecture_assessment/mod.rs (1)
The winter semester key change to "093" maintains consistency with other modules. This standardization is good for maintainability. packages/rusaint/src/application/personal_course_schedule/mod.rs (1)
The change aligns with the standardized semester key mapping across other modules. packages/rusaint/src/application/course_grades/model.rs (2)
The addition of the Empty variant properly models the case where no score is available.
The FromStr implementation correctly maps empty strings to the new Empty variant. packages/rusaint/src/application/course_grades/mod.rs (2)
The change aligns with the standardized semester key mapping across other modules.
The documentation clearly explains that negative scores indicate empty values, which helps API users understand the behavior. languages/swift/Rusaint (1)
The submodule reference has been updated to point to a new commit. Let's verify the referenced commit and its changes: Cargo.toml (1)
The version increment from 0.8.0 to 0.8.1 follows semantic versioning for bug fixes. Let's verify version consistency across the project: ✅ Verification successfulVersion consistency verified across the project The version bump to 0.8.1 is consistently reflected in both Cargo.toml and README.md, with no outdated references to version 0.8.0 remaining. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check version consistency across all relevant files
echo "Checking version numbers in project files..."
rg -l "0\.8\.1|0\.8\.0" --type toml --type md
Length of output: 167 Script: #!/bin/bash
# Check the actual version numbers in these files
echo "=== Version in Cargo.toml ==="
rg "version = \"0\.8\.[01]\"" Cargo.toml
echo -e "\n=== Version in README.md ==="
rg "0\.8\.[01]" README.md
Length of output: 275 packages/rusaint/Cargo.toml (1)
The upgrade from lol_html 1.2.1 to 2.1.0 is a major version bump that could introduce breaking changes. While this may improve HTML parsing capabilities, we should ensure it doesn't affect the course detail score handling fix. Let's check for potential breaking changes: ✅ Verification successfulBased on the shell script results, I can now make a determination about the lol_html dependency upgrade impact. The search results show that lol_html is used in a very limited way in the codebase:
Let me check the specific usage pattern to understand the impact better: Based on the detailed analysis of the codebase, I can now provide the final response: Major version upgrade of lol_html appears safe to proceed The lol_html usage is very limited and focused on HTML rewriting functionality using only stable APIs:
The upgrade from 1.2.1 to 2.1.0 is safe as the core HTML rewriting APIs being used have remained stable across major versions. The changes don't affect the course detail score handling fix. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check usage of lol_html in the codebase
echo "Checking lol_html usage patterns..."
rg "lol_html::" --type rust -A 5
# Look for any HTML parsing related to course scores
echo "Checking score-related HTML parsing..."
rg "score|grade|점수" --type rust -A 5
Length of output: 30609 Script: #!/bin/bash
# Check the specific usage of lol_html in the body module
cat packages/rusaint/src/webdynpro/client/body/mod.rs
Length of output: 10944 README.md (1)
The changes improve readability and maintain version consistency with the package updates. The Android dependency version has been correctly updated to match the new release. Also applies to: 15-16, 61-61, 65-65, 84-84 .github/workflows/release.yml (2)
The addition of the path filter for
The directory cleanup step helps prevent state pollution, but ensure it doesn't affect subsequent steps that might depend on preserved files. ✅ Verification successfulDirectory cleanup is safe and necessary The directory cleanup step is correctly placed and serves a valid purpose. It occurs between the version check and the full repository checkout, clearing any sparse-checkout artifacts. The subsequent steps are not impacted because:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if any subsequent steps in the workflow require files that might be removed
rg -A 5 "uses: actions/checkout@v4" .github/workflows/release.yml
Length of output: 518 Script: #!/bin/bash
# Let's check the entire workflow file to understand the sequence and dependencies
cat .github/workflows/release.yml
Length of output: 4061 languages/kotlin/lib/build.gradle.kts (2)
Version update to 0.8.1 is consistent with other configuration files. Line range hint Good addition of error handling for the binding generation task. This will help catch and report binding generation failures more effectively. .github/workflows/release-ios.yml (3)
The addition of directory listing helps with debugging build issues by providing visibility into the generated artifacts. 🧰 Tools🪛 actionlint (1.7.4)93-93: shellcheck reported issue in this script: SC2086:info:1:10: Double quote to prevent globbing and word splitting (shellcheck)
The codesign command has been corrected to use the proper
The release API endpoint has been updated to point to rusaint-ios repository. Ensure this is intentional and aligns with the project structure. ✅ Verification successfulAPI endpoint change is correct and intentional The API endpoint change from the previous version to the current version is consistent. Both the fetch latest release tag and create release steps use the same endpoint 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if there are any other references to the old API endpoint
rg -l "api.github.com/repos/EATSTEAK/rusaint/releases" .
Length of output: 56 Script: #!/bin/bash
# Let's check for any GitHub API release endpoints in the repository
rg -l "api.github.com/repos/.+/releases" .
# Also check the workflow file history to see the previous endpoint
git show HEAD~1:.github/workflows/release-ios.yml 2>/dev/null || echo "Previous version not found"
Length of output: 6846 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🛑 Comments failed to post (3)
packages/rusaint/src/application/lecture_assessment/mod.rs (1)
67-67: 🧹 Nitpick (assertive)
Consider centralizing semester key mapping
The
semester_to_keyfunction is duplicated across multiple modules (ChapelApplication, LectureAssessmentApplication, etc.). Consider extracting this mapping to a central location to improve maintainability and ensure consistency.Example implementation:
// src/model/semester.rs or similar impl SemesterType { pub fn to_key(&self) -> &'static str { match self { SemesterType::One => "090", SemesterType::Summer => "091", SemesterType::Two => "092", SemesterType::Winter => "093", } } }packages/rusaint/src/application/course_grades/mod.rs (1)
339-346: 🧹 Nitpick (assertive)
Consider using a constant for the empty score value
While returning -1.0 for empty scores works, consider defining it as a constant to improve maintainability and prevent magic numbers.
+const EMPTY_SCORE: f32 = -1.0; + fn parse_table_in_popup(body: &Body) -> Result<HashMap<String, f32>, WebDynproError> { // ... if str.is_empty() { - return Ok((key, -1.0)); + return Ok((key, EMPTY_SCORE)); } // ... }Committable suggestion skipped: line range outside the PR's diff.
.github/workflows/release.yml (1)
25-25:
⚠️ Potential issueFix potential shell globbing issue
The command needs proper quoting to prevent potential shell globbing and word splitting issues.
-run: yq '.workspace.package.version | "result=v" + .' Cargo.toml >> $GITHUB_OUTPUT +run: yq '.workspace.package.version | "result=v" + .' "Cargo.toml" >> "$GITHUB_OUTPUT"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.run: yq '.workspace.package.version | "result=v" + .' "Cargo.toml" >> "$GITHUB_OUTPUT"🧰 Tools
🪛 actionlint (1.7.4)
25-25: shellcheck reported issue in this script: SC2086:info:1:64: Double quote to prevent globbing and word splitting
(shellcheck)
# What's in this pull request - `CourseGradeApplication::class_detail()`에서 수업 상세 정보의 점수가 비었을 경우 음수를 반환 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated version of the `rusaint` package and its dependencies to version `0.8.1`. - Added a new `Empty` variant to the `ClassScore` enum for handling absent scores. - **Bug Fixes** - Improved error handling in the `classes_with_detail` test function to prevent panic when no matching grade is found. - Adjusted semester key mapping for `SemesterType::Winter` across multiple applications. - **Documentation** - Enhanced clarity in comments and README content, including version updates and formatting improvements. - **Chores** - Removed caching steps in workflow configurations for build, lint, and release processes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
# What's in this pull request - `CourseGradeApplication::class_detail()`에서 수업 상세 정보의 점수가 비었을 경우 음수를 반환 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated version of the `rusaint` package and its dependencies to version `0.8.1`. - Added a new `Empty` variant to the `ClassScore` enum for handling absent scores. - **Bug Fixes** - Improved error handling in the `classes_with_detail` test function to prevent panic when no matching grade is found. - Adjusted semester key mapping for `SemesterType::Winter` across multiple applications. - **Documentation** - Enhanced clarity in comments and README content, including version updates and formatting improvements. - **Chores** - Removed caching steps in workflow configurations for build, lint, and release processes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What's in this pull request
CourseGradeApplication::class_detail()에서 수업 상세 정보의 점수가 비었을 경우 음수를 반환Summary by CodeRabbit
New Features
rusaintpackage and its dependencies to version0.8.1.Emptyvariant to theClassScoreenum for handling absent scores.Bug Fixes
classes_with_detailtest function to prevent panic when no matching grade is found.SemesterType::Winteracross multiple applications.Documentation
Chores