Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: add bleeding linux #1135

Merged
merged 1 commit into from
Oct 17, 2024
Merged

ci: add bleeding linux #1135

merged 1 commit into from
Oct 17, 2024

Conversation

Young-Flash
Copy link
Contributor

No description provided.

Copy link

peter-jerry-ye-code-review bot commented Oct 17, 2024

Observed Problems in git diff Output

  1. Redundant ulimit Command:
    The ulimit -s 8176 command is repeated twice in the bleeding-check job. This command sets the stack size limit, and it is sufficient to set it once at the beginning of the script. While this does not necessarily introduce a bug, it is redundant and can be cleaned up for better readability.

  2. Potential Misuse of --release Flag:
    The --release flag is moved from its original position to be placed at the end of the command in the moon test command:

    - moon test --release --target native
    + moon test --target native --release

    While this might not be incorrect depending on how the moon tool interprets flags, it is generally good practice to place flags in a consistent order. If the tool expects --release to precede other options (like --target), this change could potentially cause issues. It's best to verify the tool's documentation or existing usage patterns to ensure flag order does not matter.

  3. Inconsistent Matrix Strategy:
    The bleeding-check job now uses a matrix strategy to run on multiple operating systems (macos-latest and ubuntu-latest). This is a good practice for ensuring cross-platform compatibility. However, if the job previously ran on only macos-latest, this change might uncover platform-specific issues that were not previously tested. It's important to ensure that all steps within the job are compatible with both operating systems and that any necessary adjustments are made to account for potential differences in behavior or tool availability.

Suggestions

  1. Remove Redundant ulimit Command:
    Consider removing the duplicate ulimit -s 8176 command to streamline the script.

  2. Verify --release Flag Position:
    Double-check the moon tool's documentation or existing usage to ensure that the position of the --release flag does not affect the command's behavior.

  3. Cross-Platform Compatibility Testing:
    Given the introduction of a matrix strategy for operating systems, thoroughly test the job on both macos-latest and ubuntu-latest to identify and resolve any platform-specific issues. This includes checking for dependencies or commands that might not be available or behave differently on different operating systems.

@coveralls
Copy link
Collaborator

coveralls commented Oct 17, 2024

Pull Request Test Coverage Report for Build 3613

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 82.303%

Totals Coverage Status
Change from base Build 3607: 0.0%
Covered Lines: 4195
Relevant Lines: 5097

💛 - Coveralls

@Young-Flash Young-Flash force-pushed the bleeding_ci_on_linux branch 4 times, most recently from f758120 to 60c9e4c Compare October 17, 2024 07:41
@Young-Flash Young-Flash merged commit 6f91a8e into main Oct 17, 2024
13 checks passed
@Young-Flash Young-Flash deleted the bleeding_ci_on_linux branch October 17, 2024 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants