Temp Fix for Windows CI #161
Conversation
WalkthroughTwo GitHub Actions workflows have been modified to include an additional step called "Get latest cibuild.cmake." In both workflows, after the initial checkout step, the action now configures the Git user settings, fetches the latest changes from the origin, and checks out specific files ( Changes
Sequence Diagram(s)sequenceDiagram
actor Runner as GitHub Runner
participant Repo as Repository
Runner->>Repo: Checkout code
Runner->>Runner: Configure Git user (name & email)
Runner->>Repo: Fetch latest changes
Runner->>Repo: Checkout files (cibuild.cmake, citest.cmake, cipackage.cmake)
Runner->>Runner: Proceed with build process
Poem
🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml (2)
21-29: Temporary Fix via Cherry-pick Commit
The newly added step cherry-picks commitccb989efrom the OpenMS repository as a temporary remedy for SeqAn-related issues until the upcoming 3.4 release. Consider enhancing the robustness of this step by adding error handling (for example, checking the exit status of the cherry-pick command) to gracefully manage any potential conflicts or failures.
30-30: Remove Trailing Whitespace
Static analysis detected trailing spaces on line 30. Please remove these to meet YAML formatting standards..github/workflows/build-windows-executable-app.yaml (2)
31-38: Temporary Cherry-pick Commit Step
The addition of the "Cherry-pick commit" step applies commitccb989efrom the OpenMS repository, serving as a temporary fix for the SeqAn downtime until OpenMS 3.4 is released. Similar to the other workflow, consider incorporating error handling to account for any failures during the cherry-pick process. Verify periodically that the commit is still valid in the targeted branch.
1-29: Inconsistent Checkout Versions
Notice that this workflow usesactions/checkout@v4, whereas the corresponding PyInstaller workflow usesactions/checkout@v3. Please confirm that this difference in checkout versions is intentional and won't lead to compatibility issues across workflows.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 8-8: trailing spaces
(trailing-spaces)
[error] 10-10: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml(1 hunks).github/workflows/build-windows-executable-app.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/build-windows-executable-app.yaml
[error] 30-30: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-simple-app
- GitHub Check: build-full-app
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build-windows-executable-app.yaml (2)
30-30: Remove trailing whitespace.
Line 30 contains trailing spaces, which violates YAMLlint standards and may lead to formatting inconsistencies. Please remove any extra trailing whitespace.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 30-30: trailing spaces
(trailing-spaces)
31-39: Implementing the temporary fix to fetch configuration files.
The newly added step correctly sets the Git configuration, fetches the latest changes from thedevelopbranch, and checks out the filestools/ci/cibuild.cmakeandtools/ci/citest.cmake. This temporary measure is appropriate given the current unavailability of SeqAn and the anticipation of the OpenMS 3.4 release.Note: While hard-coding the branch name (
develop) is acceptable for a short-term fix, please ensure that this step is revisited for stability in future releases. Additionally, consider adding error handling (if not already provided by GitHub Actions’ default behavior) to catch any issues during the file checkout process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml(1 hunks).github/workflows/build-windows-executable-app.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-windows-executable-app-with-pyinstaller.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/build-windows-executable-app.yaml
[error] 30-30: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-full-app
- GitHub Check: build-simple-app
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build-windows-executable-app.yaml (2)
30-30: Remove Trailing Whitespace.
YAMLlint detected trailing whitespace on this line. Please remove any extraneous spaces to ensure consistency with YAML style guidelines.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 30-30: trailing spaces
(trailing-spaces)
31-39: Temporary Fix Step for Configuration Files.
The newly added "Get latest cibuild.cmake" step correctly configures Git, fetches from thedevelopbranch, and checks out the required configuration files. This serves as a practical temporary solution until OpenMS 3.4 is released and the SeqAn issue is resolved. Please ensure that this step is revisited and removed or refactored when the permanent fix is implemented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml(1 hunks).github/workflows/build-windows-executable-app.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-windows-executable-app-with-pyinstaller.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/build-windows-executable-app.yaml
[error] 30-30: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-openms
- GitHub Check: build-full-app
- GitHub Check: build-simple-app
|
@jcharkow Any idea why the gui tests would be failing? |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build-windows-executable-app.yaml (2)
30-30: Remove Trailing SpacesLine 30 contains trailing spaces, as flagged by YAMLlint. Removing these extra whitespaces improves overall file cleanliness and avoids potential linting issues.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 30-30: trailing spaces
(trailing-spaces)
31-40: Temporary CI Configuration UpdateThis new step fetches and checks out updated CI configuration files (
cibuild.cmake,citest.cmake, andcipackage.cmake) from theorigin/developbranch. Since this is a temporary workaround until SeqAn is back online or a new OpenMS release (3.4) is available, please ensure to schedule its removal or proper revision in the future. As an enhancement, consider pinning the checkout to a specific commit or tag to avoid unexpected changes from the develop branch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml(1 hunks).github/workflows/build-windows-executable-app.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-windows-executable-app-with-pyinstaller.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/build-windows-executable-app.yaml
[error] 30-30: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-openms
- GitHub Check: build-simple-app
- GitHub Check: build-full-app
|
Looks like the GUI tests are failing intermittently because of a time out. I will open an issue for this. |
Cdash on seqan is currently not working. This PR fixes this temporarily by applying the fix from the main repository (will only be included in the openms 3.4 release).
Summary by CodeRabbit