[ci] iOS 배포 워크플로 등록 #146
[ci] iOS 배포 워크플로 등록 #146
Conversation
[fix] 전체 테이블을 가져오지 못한 경우 스크롤하여 파싱
[ci] Workflow 실행 시 캐시 사용, 린터 체크
[ci] iOS 배포 워크플로
EATSTEAK
added
enhancement
WalkthroughThis pull request introduces significant enhancements to the project's GitHub Actions workflows, application logic, and utility functions. The changes focus on improving build processes, implementing asynchronous data retrieval, and streamlining code handling for SAP table interactions. Key modifications include adding new workflows for linting and iOS release, updating existing workflows with caching and concurrency mechanisms, and introducing a new utility function Changes
Sequence DiagramsequenceDiagram
participant Client as USaintClient
participant Utils as SapTableUtils
participant Parser as ElementParser
participant Table as SAPTable
Client->>Utils: try_table_into_with_scroll()
Utils->>Table: Read row count
Table-->>Utils: Return row count
Utils->>Table: Read table body
loop Until desired results
Utils->>Table: Read partial results
Table-->>Utils: Return partial data
alt Need more data
Utils->>Table: Scroll vertically
end
end
Utils-->>Client: Return processed results
Possibly related PRs
Suggested Labels
Poem
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
|
[ci] `dry-run` 워크플로 수정
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🔭 Outside diff range comments (1)
packages/rusaint/src/webdynpro/client/mod.rs (1)
Line range hint
124-127: Improve error handling in signing step.The code signing step could have more robust error handling:
-security unlock-keychain -p "$KEYCHAIN_PASSWORD" $RUNNER_TEMP/signing.keychain-db -codesign --timestam-p -s "Apple Development" ./languages/swift/Rusaint/Artifacts/RusaintFFI.xcframework +if ! security unlock-keychain -p "${KEYCHAIN_PASSWORD}" "${RUNNER_TEMP}/signing.keychain-db"; then + echo "Failed to unlock keychain" + exit 1 +fi +if ! codesign --timestamp -s "Apple Development" ./languages/swift/Rusaint/Artifacts/RusaintFFI.xcframework; then + echo "Failed to sign framework" + exit 1 +fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (16)
.github/workflows/build_test.yml(3 hunks).github/workflows/dry-run.yml(2 hunks).github/workflows/lint.yml(1 hunks).github/workflows/release-ios.yml(1 hunks).github/workflows/release.yml(2 hunks)languages/swift/Rusaint(1 hunks)languages/swift/build.sh(1 hunks)packages/rusaint/src/application/course_grades/mod.rs(2 hunks)packages/rusaint/src/application/course_schedule/mod.rs(2 hunks)packages/rusaint/src/application/lecture_assessment/mod.rs(5 hunks)packages/rusaint/src/application/student_information/model/academic_record.rs(2 hunks)packages/rusaint/src/application/student_information/model/family.rs(2 hunks)packages/rusaint/src/application/student_information/model/transfer.rs(2 hunks)packages/rusaint/src/application/utils/mod.rs(1 hunks)packages/rusaint/src/application/utils/sap_table.rs(1 hunks)packages/rusaint/src/webdynpro/client/mod.rs(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/build_test.yml
[error] 6-6: too many spaces inside brackets
(brackets)
[error] 6-6: too many spaces inside brackets
(brackets)
.github/workflows/lint.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 6-6: too many spaces inside brackets
(brackets)
[error] 6-6: too many spaces inside brackets
(brackets)
[error] 23-23: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/release-ios.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[error] 150-150: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.4)
.github/workflows/release.yml
28-28: shellcheck reported issue in this script: SC2086:info:7:18: Double quote to prevent globbing and word splitting
(shellcheck)
51-51: "with" section should not be empty. please remove this section if it's unnecessary
(syntax-check)
.github/workflows/release-ios.yml
29-29: shellcheck reported issue in this script: SC2086:info:7:18: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:6:58: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:9:50: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:10:43: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:11:50: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:14:17: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:14:78: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:15:79: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:16:35: Double quote to prevent globbing and word splitting
(shellcheck)
84-84: shellcheck reported issue in this script: SC2086:info:1:7: Double quote to prevent globbing and word splitting
(shellcheck)
84-84: shellcheck reported issue in this script: SC2034:warning:2:1: CARGO_TARGET_DIR appears unused. Verify use (or export if used externally)
(shellcheck)
91-91: shellcheck reported issue in this script: SC2086:info:1:10: Double quote to prevent globbing and word splitting
(shellcheck)
91-91: shellcheck reported issue in this script: SC2086:info:3:3: Double quote to prevent globbing and word splitting
(shellcheck)
91-91: shellcheck reported issue in this script: SC2086:info:4:3: Double quote to prevent globbing and word splitting
(shellcheck)
91-91: shellcheck reported issue in this script: SC2086:info:5:11: Double quote to prevent globbing and word splitting
(shellcheck)
98-98: shellcheck reported issue in this script: SC2086:info:2:3: Double quote to prevent globbing and word splitting
(shellcheck)
98-98: shellcheck reported issue in this script: SC2086:info:6:13: Double quote to prevent globbing and word splitting
(shellcheck)
106-106: shellcheck reported issue in this script: SC2086:info:1:4: Double quote to prevent globbing and word splitting
(shellcheck)
108-108: shellcheck reported issue in this script: SC2086:info:1:7: Double quote to prevent globbing and word splitting
(shellcheck)
108-108: shellcheck reported issue in this script: SC2086:info:2:4: Double quote to prevent globbing and word splitting
(shellcheck)
108-108: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
108-108: shellcheck reported issue in this script: SC2086:info:3:5: Double quote to prevent globbing and word splitting
(shellcheck)
108-108: shellcheck reported issue in this script: SC2086:info:3:41: Double quote to prevent globbing and word splitting
(shellcheck)
113-113: shellcheck reported issue in this script: SC2086:info:3:14: Double quote to prevent globbing and word splitting
(shellcheck)
113-113: shellcheck reported issue in this script: SC2086:info:4:14: Double quote to prevent globbing and word splitting
(shellcheck)
113-113: shellcheck reported issue in this script: SC2086:info:5:14: Double quote to prevent globbing and word splitting
(shellcheck)
113-113: shellcheck reported issue in this script: SC2086:info:6:14: Double quote to prevent globbing and word splitting
(shellcheck)
124-124: shellcheck reported issue in this script: SC2086:info:1:50: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (21)
packages/rusaint/src/application/utils/sap_table.rs (1)
31-38: Validate partial results trimming logic.
Here, you conditionally drain the overflowed items using partial_results.drain(0..overflowed). Confirm that draining from index 0 is indeed the intended behavior. If the results array is already partially populated, you might need to remove the extra items from the end of partial_results instead.
packages/rusaint/src/application/lecture_assessment/mod.rs (2)
Line range hint 145-155: Ensure consistent error handling for empty table scenarios.
When row_count is 0 or missing, an ElementError::NoSuchData is returned. Verify that this scenario is truly an error, or if returning an empty vector might be acceptable.
173-173: Seamless integration with try_table_into_with_scroll.
The one-liner usage of try_table_into_with_scroll neatly replaces manual looping logic. Make sure to thoroughly test if additional fields within LectureAssessmentResult require further parsing outside the table scroll logic.
packages/rusaint/src/application/course_grades/mod.rs (2)
3-3: Don't forget to handle the error boundary.
When the try_table_into_with_scroll function is invoked, ensure that errors raised by the scrolling logic are properly handled or logged in case of partial fetches or timeouts.
299-299: Asynchronous method call chain recognized.
Switching to an async call for read_semesters is consistent with the rest of the codebase changes. Confirm that any callers of this method are also updated to handle async.
languages/swift/Rusaint (1)
1-1: 🧹 Nitpick (assertive)
Verify the submodule update for iOS deployment.
The submodule reference has been updated to a new commit. Since this PR aims to register iOS deployment workflow, please ensure that:
- The referenced commit (42d58e1) is stable and tested
- It contains the necessary iOS-specific changes
Let's verify the commit changes:
Consider adding a comment in the commit message or PR description explaining what specific changes this submodule update brings in, particularly regarding iOS deployment configuration.
✅ Verification successful
The initial results show the existence of iOS project files, but we need more context about the workflow registration and submodule state to make a proper assessment.
The presence of release-ios.yml with deployment configuration suggests this is part of a broader iOS deployment setup. Let's verify the workflow details and related configuration to ensure the submodule update aligns with the deployment requirements.
Submodule update is aligned with iOS deployment workflow
The submodule update is verified to be part of a well-structured iOS deployment process:
- The
release-ios.ymlworkflow handles the complete iOS build and release pipeline - The submodule is automatically updated as part of the release process (lines 127-134)
- The workflow includes proper version checking, build configuration, and code signing
- The submodule update is consistent with the automated release process
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the submodule commit changes
# Note: This will help understand what changes are being pulled in
# Check if the submodule commit exists and get its details
git ls-remote | grep 42d58e1de286cb2f5ef9f9a18e79f64198323e2c
# Look for iOS-related configuration files in the repository
fd -t f -e xcodeproj -e xcworkspace -e plist
Length of output: 263
Script:
#!/bin/bash
# Let's verify the submodule changes more thoroughly
# Check the current state of the submodule
git submodule status languages/swift/Rusaint
# Look for iOS deployment workflow files
fd -t f workflow .github/workflows/
# Check for any iOS build or deployment related files
rg -i "ios.*deploy|deploy.*ios" .github/workflows/
Length of output: 319
Script:
#!/bin/bash
# Let's check the iOS release workflow content and related configuration
# Check the iOS release workflow content
cat .github/workflows/release-ios.yml
# Check if there are any other files referencing the submodule
rg -l "Rusaint" .github/workflows/
# Look for any build configuration files that might be affected
fd -t f -e xcconfig -e podspec languages/swift/
Length of output: 7107
packages/rusaint/src/application/utils/mod.rs (1)
3-3: Verify if this change belongs in this PR
This change introduces a new SAP table utility module, which seems unrelated to the PR's objective of registering an iOS deployment workflow. Consider moving this change to a separate PR focused on SAP table functionality improvements.
.github/workflows/lint.yml (1)
1-23: Verify if this workflow belongs in this PR
This new lint workflow appears to be focused on Rust code quality, which seems unrelated to the PR's objective of registering an iOS deployment workflow. Consider:
- Moving this to a separate PR focused on Rust tooling improvements
- Adding a final newline to the file
- Adding iOS-specific linting if this PR is intended to support iOS deployment
✅ Verification successful
The Rust linting workflow is appropriate for this repository
This is a Rust-heavy project with over 100 Rust source files across multiple packages (rusaint, rusaint-ffi). While the PR mentions iOS deployment, the repository contains a significant Rust codebase that needs proper linting. The lint workflow:
- Ensures code quality across the Rust codebase that powers both the iOS and other platform implementations
- Complements the iOS-specific workflows by maintaining the quality of the underlying Rust code
- Is correctly configured with necessary Rust tooling (clippy, rustfmt)
Minor suggestions:
- Add a final newline to the workflow file
- Consider adding iOS-specific linting in a separate workflow file if needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any iOS-specific workflow files that might need coordination with this lint workflow
fd -e yml -e yaml . .github/workflows/ --exec grep -l "ios"
Length of output: 213
Script:
#!/bin/bash
# Check the content of iOS-related workflow files to understand the context
for file in .github/workflows/{dry-run,lint,build_test,release-ios,release}.yml; do
echo "=== $file ==="
cat "$file"
echo -e "\n\n"
done
Length of output: 13797
Script:
#!/bin/bash
# Check if this is a Rust project and if there are Rust files that need linting
fd -e rs . --type f
# Also check the PR description or title to understand the context
gh pr view $PR_NUMBER
Length of output: 7361
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 6-6: too many spaces inside brackets
(brackets)
[error] 6-6: too many spaces inside brackets
(brackets)
[error] 23-23: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/dry-run.yml (3)
9-14: Well-implemented concurrency control
The concurrency configuration is well-structured and will help prevent resource conflicts by canceling in-progress runs.
25-26: Cache implementation matches other workflows
The cache retrieval step is consistent with other workflows in the repository.
Line range hint 1-30: Missing iOS deployment workflow
Given that this PR's objective is to register an iOS deployment workflow, I notice that:
- The changes in this file focus on Rust package publishing
- There's no iOS-specific workflow file added in this PR
Consider adding a new workflow file specifically for iOS deployment that includes:
- iOS build configuration
- Code signing setup
- App Store deployment steps
.github/workflows/build_test.yml (2)
15-17: LGTM: Concurrency control added
Good addition of concurrency control to prevent redundant builds and save CI resources.
27-30: LGTM: Build caching implemented
Good addition of cargo caching which will speed up builds significantly.
languages/swift/build.sh (1)
24-24: LGTM: Correct target architecture for iOS deployment
The change to use aarch64-apple-ios target for generating Swift bindings is correct for iOS deployment. The script properly handles universal library creation and xcframework generation.
✅ Verification successful
LGTM: Verified iOS target architecture configuration
The build script correctly handles multiple iOS architectures:
- Uses
aarch64-apple-iosfor physical iOS devices - Uses
aarch64-apple-ios-simandx86_64-apple-iosfor simulators - Creates a universal binary using
lipofor simulator support - Properly generates xcframework with both device and simulator architectures
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify Rust targets are installed
rustup target list | grep -E "aarch64-apple-ios|x86_64-apple-ios"
# Check Cargo.toml for iOS-specific configurations
rg "^\[target.*ios\]" -A 5
Length of output: 193
Script:
#!/bin/bash
# Check if the iOS target directory exists in the Rust build output
fd "aarch64-apple-ios" target/
# Look for any iOS related build configurations or target specifications
rg -i "ios|apple" build.sh languages/swift/build.sh
# Check for any Rust target configurations in config files
fd -e toml | xargs rg -i "ios|apple"
Length of output: 1181
packages/rusaint/src/application/student_information/model/academic_record.rs (1)
8-8: LGTM: Improved table data handling
Good improvement using try_table_into_with_scroll for better SAP table handling with scroll support.
Also applies to: 51-53
✅ Verification successful
Function usage is consistent across the codebase
The verification shows that try_table_into_with_scroll is properly used across multiple modules with consistent error handling patterns. All usages follow the same pattern of passing client, parser, and table name parameters with proper error propagation using the ? operator.
Found in:
- academic_record.rs (student records)
- family.rs (family members)
- transfer.rs (transfer records)
- course_schedule/mod.rs (lectures)
- course_grades/mod.rs (semester grades)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check usage of try_table_into_with_scroll across codebase
echo "Checking usage of new table scroll function:"
rg "try_table_into_with_scroll" -l
# Verify error handling patterns
echo "Checking error handling patterns:"
ast-grep --pattern 'try_table_into_with_scroll::<$_>($_,$_,$_).await?'
Length of output: 2661
packages/rusaint/src/application/student_information/model/transfer.rs (1)
8-8: LGTM! Improved table data handling with scroll support
The refactoring to use try_table_into_with_scroll is a good improvement that:
- Provides consistent table data handling across modules
- Adds scroll support for handling large datasets
- Maintains existing error handling
Also applies to: 51-56
.github/workflows/release.yml (1)
14-42: LGTM! Robust version management implementation
The new version management steps effectively prevent unnecessary releases by:
- Checking current version from Cargo.toml
- Comparing with latest release tag
- Canceling workflow if versions match
🧰 Tools
🪛 actionlint (1.7.4)
28-28: shellcheck reported issue in this script: SC2086:info:7:18: Double quote to prevent globbing and word splitting
(shellcheck)
packages/rusaint/src/application/course_schedule/mod.rs (1)
2-2: LGTM! Consistent table handling implementation
The refactoring aligns with the project-wide improvements in table data handling:
- Uses the same utility function as other modules
- Maintains existing error handling
- Improves handling of large lecture datasets
Also applies to: 132-134
packages/rusaint/src/application/student_information/model/family.rs (3)
9-9: LGTM: Import addition is appropriate
The new import for try_table_into_with_scroll is correctly placed and necessary for the updated implementation.
52-54: LGTM: Clean implementation of async table processing
The change to use try_table_into_with_scroll simplifies the code while maintaining proper error handling. The async implementation should improve responsiveness.
52-54: Verify performance with large datasets
Since this implementation handles scrolling, it's important to verify its behavior with varying table sizes.
✅ Verification successful
Based on the code analysis, I can now provide a final response regarding the scrolling implementation:
Implementation has built-in safeguards for handling large datasets
The try_table_into_with_scroll function is well-designed with proper pagination handling:
- It uses SAP WebDynpro's native scrolling mechanism through
SapTableVerticalScrollEventCommand - The implementation automatically handles chunked data loading by:
- Tracking total row count
- Incrementally fetching data until all rows are retrieved
- Using proper scroll events with first visible item index tracking
- The scrolling behavior is standardized across different table usages (family members, academic records, course grades, etc.)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for test cases or benchmarks related to table scrolling
rg -g '*.rs' -A 5 'try_table_into_with_scroll.*test|bench'
# Look for any existing performance considerations or documentation
rg -g '*.rs' -A 5 'try_table_into_with_scroll.*TODO|PERF|NOTE'
Length of output: 125
Script:
#!/bin/bash
# Let's first look at the implementation of try_table_into_with_scroll
ast-grep --pattern 'fn try_table_into_with_scroll'
# Search for any tests in the same directory or test directories
fd -e rs test family
fd -e rs family_test
# Look for any documentation or comments about scrolling behavior
rg -g '*.rs' -B 2 -A 5 'scroll|pagination' --ignore-case
Length of output: 34268
| pub(crate) async fn try_table_into_with_scroll<T: for<'body> FromSapTable<'body>>( | ||
| client: &mut USaintClient, | ||
| mut parser: ElementParser, | ||
| table: SapTableDef, | ||
| ) -> Result<Vec<T>, WebDynproError> { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure consistent error messages.
The function name and signature are clear. However, consider clarifying in the docstring or error messages that “scroll” refers to fetching additional table data from SAP. This will help developers quickly understand the function’s purpose.
| let event = parser.read(SapTableVerticalScrollEventCommand::new( | ||
| table.clone(), | ||
| results.len().try_into().unwrap(), | ||
| "", | ||
| "SCROLLBAR", | ||
| false, | ||
| false, | ||
| false, | ||
| false, | ||
| ))?; | ||
| client.process_event(false, event).await?; | ||
| parser = ElementParser::new(client.body()); | ||
| table_body = parser.read(SapTableBodyCommand::new(table.clone()))?; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optimize repeated scrolling procedure.
The vertical scroll logic inside this while loop is precise but somewhat verbose. Consider abstracting it into a helper function if scrolling is used in multiple parts of the code, improving reusability and readability.
| } | ||
| Ok(results) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Include usage details for the caller.
Returning an Ok(results) is correct, but it might be useful to add a short docstring note on how the resulting Vec is populated with partial scroll segments before final retrieval. This can help other developers quickly glean how the asynchronous scrolling works.
| @@ -1,6 +1,7 @@ | |||
| use model::LectureAssessmentResult; | |||
|
|
|||
| use super::{USaintApplication, USaintClient}; | |||
| use crate::application::utils::sap_table::try_table_into_with_scroll; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Replace direct call with a re-export if used frequently.
If try_table_into_with_scroll is used in multiple modules, consider exporting it from a more central location (e.g., prelude, mod.rs) to reduce repeated import paths and improve discoverability.
| .unwrap(); | ||
| let mut table = parser.read(SapTableBodyCommand::new(Self::TABLE))?; | ||
| })?; | ||
| let table = parser.read(SapTableBodyCommand::new(Self::TABLE))?; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider deferring table read.
Reading the table here (table = parser.read(...)) is valid. However, if subsequent logic can't proceed without row_count > 0, you may want to conditionally create or read the table only when needed, to save overhead.
| on: | ||
| push: | ||
| branches: [ dev ] | ||
| paths: | ||
| - './Cargo.toml' | ||
| workflow_dispatch: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding concurrency control to prevent parallel releases.
Add a concurrency configuration to ensure only one release workflow runs at a time:
on:
push:
branches: [ dev ]
paths:
- './Cargo.toml'
workflow_dispatch:
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true📝 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.
| on: | |
| push: | |
| branches: [ dev ] | |
| paths: | |
| - './Cargo.toml' | |
| workflow_dispatch: | |
| on: | |
| push: | |
| branches: [ dev ] | |
| paths: | |
| - './Cargo.toml' | |
| workflow_dispatch: | |
| concurrency: | |
| group: ${{ github.workflow }}-${{ github.ref }} | |
| cancel-in-progress: true |
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
| - name: Create release | ||
| run: | | ||
| curl -L \ | ||
| -X POST \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| -H "Authorization: Bearer ${{ secrets.IOS_GITHUB_TOKEN }}" \ | ||
| -H "X-GitHub-Api-Version: 2022-11-28" \ | ||
| https://api.github.com/repos/EATSTEAK/rusaint-ios/releases \ | ||
| -d '{"tag_name":"${{ steps.current_version.outputs.result }}","name":"${{ steps.current_version.outputs.result }}"}' |
There was a problem hiding this comment.
Enhance release creation error handling.
The release creation step lacks error handling and validation:
-curl -L \
- -X POST \
- -H "Accept: application/vnd.github+json" \
- -H "Authorization: Bearer ${{ secrets.IOS_GITHUB_TOKEN }}" \
- -H "X-GitHub-Api-Version: 2022-11-28" \
- https://api.github.com/repos/EATSTEAK/rusaint-ios/releases \
- -d '{"tag_name":"${{ steps.current_version.outputs.result }}","name":"${{ steps.current_version.outputs.result }}"}'
+response=$(curl -L \
+ -X POST \
+ -H "Accept: application/vnd.github+json" \
+ -H "Authorization: Bearer ${{ secrets.IOS_GITHUB_TOKEN }}" \
+ -H "X-GitHub-Api-Version: 2022-11-28" \
+ https://api.github.com/repos/EATSTEAK/rusaint-ios/releases \
+ -d "{\"tag_name\":\"${{ steps.current_version.outputs.result }}\",\"name\":\"${{ steps.current_version.outputs.result }}\"}" \
+ -w "%{http_code}")
+if [[ "${response}" != 2* ]]; then
+ echo "Failed to create release: ${response}"
+ exit 1
+fi📝 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.
| - name: Create release | |
| run: | | |
| curl -L \ | |
| -X POST \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "Authorization: Bearer ${{ secrets.IOS_GITHUB_TOKEN }}" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| https://api.github.com/repos/EATSTEAK/rusaint-ios/releases \ | |
| -d '{"tag_name":"${{ steps.current_version.outputs.result }}","name":"${{ steps.current_version.outputs.result }}"}' | |
| - name: Create release | |
| run: | | |
| response=$(curl -L \ | |
| -X POST \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "Authorization: Bearer ${{ secrets.IOS_GITHUB_TOKEN }}" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| https://api.github.com/repos/EATSTEAK/rusaint-ios/releases \ | |
| -d "{\"tag_name\":\"${{ steps.current_version.outputs.result }}\",\"name\":\"${{ steps.current_version.outputs.result }}\"}" \ | |
| -w "%{http_code}") | |
| if [[ "${response}" != 2* ]]; then | |
| echo "Failed to create release: ${response}" | |
| exit 1 | |
| fi | |
🧰 Tools
🪛 yamllint (1.35.1)
[error] 150-150: no new line character at the end of file
(new-line-at-end-of-file)
| env: | ||
| IPHONEOS_DEPLOYMENT_TARGET: ${{ vars.IOS_DEPLOYMENT_TARGET || '14.0' }} | ||
| RUSTFLAGS: '-C link-arg=-Wl,-application_extension' | ||
| run: | | ||
| mkdir $RUNNER_TEMP/target | ||
| CARGO_TARGET_DIR=$RUNNER_TEMP/target | ||
| cargo build --package rusaint-ffi --target aarch64-apple-ios-sim --release | ||
| cargo build --package rusaint-ffi --target aarch64-apple-ios --release | ||
| cargo build --package rusaint-ffi --target x86_64-apple-ios --release |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve build configuration and error handling.
The native library build step could be enhanced:
env:
IPHONEOS_DEPLOYMENT_TARGET: ${{ vars.IOS_DEPLOYMENT_TARGET || '14.0' }}
RUSTFLAGS: '-C link-arg=-Wl,-application_extension'
run: |
- mkdir $RUNNER_TEMP/target
- CARGO_TARGET_DIR=$RUNNER_TEMP/target
- cargo build --package rusaint-ffi --target aarch64-apple-ios-sim --release
- cargo build --package rusaint-ffi --target aarch64-apple-ios --release
- cargo build --package rusaint-ffi --target x86_64-apple-ios --release
+ mkdir -p "${RUNNER_TEMP}/target"
+ export CARGO_TARGET_DIR="${RUNNER_TEMP}/target"
+ targets=("aarch64-apple-ios-sim" "aarch64-apple-ios" "x86_64-apple-ios")
+ for target in "${targets[@]}"; do
+ echo "Building for ${target}..."
+ cargo build --package rusaint-ffi --target "${target}" --release || exit 1
+ done📝 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.
| env: | |
| IPHONEOS_DEPLOYMENT_TARGET: ${{ vars.IOS_DEPLOYMENT_TARGET || '14.0' }} | |
| RUSTFLAGS: '-C link-arg=-Wl,-application_extension' | |
| run: | | |
| mkdir $RUNNER_TEMP/target | |
| CARGO_TARGET_DIR=$RUNNER_TEMP/target | |
| cargo build --package rusaint-ffi --target aarch64-apple-ios-sim --release | |
| cargo build --package rusaint-ffi --target aarch64-apple-ios --release | |
| cargo build --package rusaint-ffi --target x86_64-apple-ios --release | |
| env: | |
| IPHONEOS_DEPLOYMENT_TARGET: ${{ vars.IOS_DEPLOYMENT_TARGET || '14.0' }} | |
| RUSTFLAGS: '-C link-arg=-Wl,-application_extension' | |
| run: | | |
| mkdir -p "${RUNNER_TEMP}/target" | |
| export CARGO_TARGET_DIR="${RUNNER_TEMP}/target" | |
| targets=("aarch64-apple-ios-sim" "aarch64-apple-ios" "x86_64-apple-ios") | |
| for target in "${targets[@]}"; do | |
| echo "Building for ${target}..." | |
| cargo build --package rusaint-ffi --target "${target}" --release || exit 1 | |
| done |
🧰 Tools
🪛 actionlint (1.7.4)
84-84: shellcheck reported issue in this script: SC2086:info:1:7: Double quote to prevent globbing and word splitting
(shellcheck)
84-84: shellcheck reported issue in this script: SC2034:warning:2:1: CARGO_TARGET_DIR appears unused. Verify use (or export if used externally)
(shellcheck)
| run: | | ||
| # create variables | ||
| CERTIFICATE_PATH=$RUNNER_TEMP/build_certificate.p12 | ||
| KEYCHAIN_PATH=$RUNNER_TEMP/signing.keychain-db | ||
|
|
||
| # import certificate from secrets | ||
| echo -n "$BUILD_CERTIFICATE_BASE64" | base64 --decode -o $CERTIFICATE_PATH | ||
|
|
||
| # create temporary keychain | ||
| security create-keychain -p "$KEYCHAIN_PASSWORD" $KEYCHAIN_PATH | ||
| security set-keychain-settings -lut 21600 $KEYCHAIN_PATH | ||
| security unlock-keychain -p "$KEYCHAIN_PASSWORD" $KEYCHAIN_PATH | ||
|
|
||
| # import certificate to keychain | ||
| security import $CERTIFICATE_PATH -P "$P12_PASSWORD" -A -t cert -f pkcs12 -k $KEYCHAIN_PATH | ||
| security set-key-partition-list -S apple-tool:,apple: -k "$KEYCHAIN_PASSWORD" $KEYCHAIN_PATH | ||
| security list-keychain -d user -s $KEYCHAIN_PATH |
There was a problem hiding this comment.
Enhance keychain security practices.
The keychain handling script has several security considerations:
- Paths are not properly quoted
- Temporary files should be cleaned up
- Keychain settings could be more secure
Apply this diff to improve security:
-CERTIFICATE_PATH=$RUNNER_TEMP/build_certificate.p12
-KEYCHAIN_PATH=$RUNNER_TEMP/signing.keychain-db
+CERTIFICATE_PATH="${RUNNER_TEMP}/build_certificate.p12"
+KEYCHAIN_PATH="${RUNNER_TEMP}/signing.keychain-db"
+trap 'rm -f "${CERTIFICATE_PATH}" && security delete-keychain "${KEYCHAIN_PATH}"' EXIT
-security create-keychain -p "$KEYCHAIN_PASSWORD" $KEYCHAIN_PATH
-security set-keychain-settings -lut 21600 $KEYCHAIN_PATH
-security unlock-keychain -p "$KEYCHAIN_PASSWORD" $KEYCHAIN_PATH
+security create-keychain -p "${KEYCHAIN_PASSWORD}" "${KEYCHAIN_PATH}"
+security set-keychain-settings -lut 3600 -t 3600 "${KEYCHAIN_PATH}"
+security unlock-keychain -p "${KEYCHAIN_PASSWORD}" "${KEYCHAIN_PATH}"Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.4)
63-63: shellcheck reported issue in this script: SC2086:info:6:58: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:9:50: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:10:43: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:11:50: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:14:17: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:14:78: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:15:79: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: shellcheck reported issue in this script: SC2086:info:16:35: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
| @@ -38,7 +38,7 @@ fn wd_xhr_header() -> HeaderMap { | |||
| headers | |||
| } | |||
|
|
|||
| impl<'a> WebDynproClient { | |||
| impl WebDynproClient { | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding documentation for lifetime changes.
The removal of lifetime parameter 'a from impl WebDynproClient might affect how the struct is used.
Add a documentation comment explaining the lifetime changes and their implications:
/// Implementation of WebDynproClient methods.
/// Note: Previously had a lifetime parameter which was removed to simplify ownership semantics.
impl WebDynproClient {
What's in this pull request
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Chores