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

commit webvoyager result md #1567

Merged
merged 1 commit into from
Jan 15, 2025
Merged

commit webvoyager result md #1567

merged 1 commit into from
Jan 15, 2025

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Jan 15, 2025

Important

Increase maximum allowed file size from 5000KB to 15000KB in pre-commit configuration.

  • Configuration:
    • Increase --maxkb argument from 5000 to 15000 in check-added-large-files hook in .pre-commit-config.yaml.

This description was created by Ellipsis for 4e99ab7. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to ff2e05d in 8 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 1 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .pre-commit-config.yaml:8
  • Draft comment:
    Increasing the max file size to 15000KB may lead to performance issues during pre-commit checks if large files are committed frequently. Consider if this increase is necessary and if it aligns with the project's needs.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change increases the maximum allowed file size for pre-commit checks. This could lead to performance issues if very large files are committed frequently.

Workflow ID: wflow_IICbd3rWD8NlUl9S


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@wintonzheng wintonzheng force-pushed the shu/add_eval_result_to_oss branch from ff2e05d to c902d8b Compare January 15, 2025 21:15
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on c902d8b in 36 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .pre-commit-config.yaml:8
  • Draft comment:
    Increasing the max file size limit to 15000KB may lead to performance issues if large files are committed frequently. Consider if this increase is necessary and if it aligns with your project's needs.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is speculative about performance issues. It's asking the author to "consider" something rather than pointing out a concrete problem. The presence of video file exclusions suggests the team has already thought about large file handling. The change appears intentional and considered.
    The comment might be raising a valid concern about repository bloat over time. Large files in git history can cause long-term issues.
    While repository size is a valid concern, this is a configuration choice that was likely made with awareness of the tradeoffs. The comment doesn't provide concrete evidence of a problem.
    Delete the comment as it's speculative, asks for consideration rather than pointing out a concrete issue, and questions an intentional configuration choice.

Workflow ID: wflow_8HxibNQYRhq5InfB


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@wintonzheng wintonzheng force-pushed the shu/add_eval_result_to_oss branch from c902d8b to 4e99ab7 Compare January 15, 2025 22:15
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on 4e99ab7 in 38 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .pre-commit-config.yaml:8
  • Draft comment:
    Increasing the max file size limit to 15000KB may lead to performance issues during pre-commit checks if large files are frequently committed. Consider whether this increase is necessary and if it aligns with your project's needs.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is speculative ("may lead to performance issues") and doesn't provide concrete evidence of a problem. The presence of exclude patterns suggests the team has already thought about file size management. The change appears intentional and the team likely has a specific reason for increasing the limit.
    I might be underestimating the importance of pre-commit performance. Large files could significantly slow down the git workflow for all developers.
    While performance is important, this comment is still speculative and asks for reconsideration rather than pointing out a concrete issue. The exclude pattern shows the team is already managing large files thoughtfully.
    Delete the comment as it's speculative, questions the author's intention, and doesn't point out a concrete issue that needs fixing.

Workflow ID: wflow_RkWTM0aTrEaUOjLv


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@wintonzheng wintonzheng merged commit 80f136c into main Jan 15, 2025
6 checks passed
@wintonzheng wintonzheng deleted the shu/add_eval_result_to_oss branch January 15, 2025 22:20
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.

1 participant