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: merge releases and extract format/info check #1115

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

peter-jerry-ye
Copy link
Collaborator

@peter-jerry-ye peter-jerry-ye commented Oct 12, 2024

Merge check/test of latest/nightly/bleeding together using matrix to avoid duplication.

Extract format and interface check to separate tasks to make it more clear what actually failed.

TODO: update branch protection rules after merge.

Copy link

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

Suggestions on Problems in git diff

  1. Unnecessary Continuation on Error for bleeding Release on Windows:

    • The diff shows that the bleeding release is excluded from running on windows-latest in the matrix strategy. This is a sensible precaution due to potential compatibility issues with the bleeding-edge release. However, the continue-on-error for the bleeding release on windows-latest is set to true, which might mask critical errors that should be addressed for future compatibility.
    • Suggestion: Consider removing the continue-on-error: true for the bleeding release on windows-latest to ensure that any issues are properly flagged and addressed.
  2. Inconsistent Handling of ulimit Across OSs:

    • The diff indicates that ulimit -s 8176 is set for non-Windows OSs but not for Windows. This might lead to inconsistencies in resource allocation and performance testing across different platforms.
    • Suggestion: Ensure that resource limits are consistently applied across all operating systems. For Windows, consider using a comparable mechanism to set resource limits if available, or document the reason for not applying it on Windows.
  3. Deprecated git diff Usage in format-check:

    • The diff includes a step to run git diff after formatting checks. This approach can be inefficient and might not accurately reflect the changes made by the formatter, especially if multiple files are involved.
    • Suggestion: Consider using a more targeted approach to identify and report formatting issues, such as comparing the output of moon fmt --check directly to the expected formatting rules. This could provide more precise feedback and streamline the CI process.

These observations aim to improve the robustness, consistency, and efficiency of the CI workflow across different operating systems and release types.

@coveralls
Copy link
Collaborator

coveralls commented Oct 12, 2024

Pull Request Test Coverage Report for Build 3698

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 83.149%

Totals Coverage Status
Change from base Build 3696: 0.0%
Covered Lines: 4283
Relevant Lines: 5151

💛 - Coveralls

@peter-jerry-ye peter-jerry-ye marked this pull request as draft October 12, 2024 07:32
@peter-jerry-ye peter-jerry-ye marked this pull request as ready for review October 12, 2024 07:39
@peter-jerry-ye peter-jerry-ye force-pushed the zihang/separate-check branch 5 times, most recently from 9913793 to 8d99ac9 Compare October 12, 2024 08:09
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.

3 participants