Skip to content

[2/N] Lint enhancements #1735

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[2/N] Lint enhancements #1735

wants to merge 6 commits into from
+1,155 −585

Conversation

Potabk
Copy link
Contributor

@Potabk Potabk commented Jul 11, 2025 •
edited by github-actions bot
Loading

What this PR does / why we need it?

Step of #1241, in the pr,

  1. Enable the shellcheck check
  2. Added stricter ruff checking to maintain good coding style
  3. Made corresponding clean code

Does this PR introduce any user-facing change?

How was this patch tested?

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jul 11, 2025
@Potabk Potabk force-pushed the ci_1 branch 2 times, most recently from 7fe24f7 to e6fa107 Compare July 14, 2025 01:15
@Potabk
Copy link
Contributor Author

Potabk commented Jul 14, 2025

@wangxiyuan @Yikun This is ready for review

Yikun
Yikun approved these changes Jul 14, 2025
View reviewed changes
@Potabk
Copy link
Contributor Author

Potabk commented Jul 14, 2025

ci break for upstream change, repairing

@codecov Codecov
Copy link

codecov bot commented Jul 15, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 79.24528% with 22 lines in your changes missing coverage. Please review.

Project coverage is 71.58%. Comparing base (ff97740) to head (044b22d).

Files with missing lines Patch % Lines
vllm_ascend/attention/mla_v1.py 28.57% 5 Missing ⚠️
vllm_ascend/models/deepseek_dbo.py 37.50% 5 Missing ⚠️
vllm_ascend/quantization/quantizer.py 57.14% 3 Missing ⚠️
vllm_ascend/multistream/ms_split.py 88.23% 2 Missing ⚠️
..._ascend/distributed/device_communicators/pyhccl.py 0.00% 1 Missing ⚠️
vllm_ascend/distributed/kv_transfer/simple_pipe.py 0.00% 1 Missing ⚠️
vllm_ascend/models/deepseek_v2.py 80.00% 1 Missing ⚠️
vllm_ascend/models/qwen2_5_vl.py 66.66% 1 Missing ⚠️
vllm_ascend/quantization/func_wrapper.py 0.00% 1 Missing ⚠️
vllm_ascend/quantization/w8a8_dynamic.py 0.00% 1 Missing ⚠️
... and 1 more

❌ Your patch check has failed because the patch coverage (79.24%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1735      +/-   ##
==========================================
+ Coverage   71.49%   71.58%   +0.09%     
==========================================
  Files          86       86              
  Lines        9131     9087      -44     
==========================================
- Hits         6528     6505      -23     
+ Misses       2603     2582      -21
Flag Coverage Δ
unittests 71.58% <79.24%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Potabk
Copy link
Contributor Author

Potabk commented Jul 15, 2025

Should enable python init check after vllm-project/vllm#20908 merged

@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Potabk Potabk force-pushed the ci_1 branch 2 times, most recently from 044b22d to 8da6754 Compare July 24, 2025 12:29
Yikun
Yikun reviewed Jul 25, 2025
View reviewed changes
Copy link
Collaborator

@Yikun Yikun left a comment

Choose a reason for hiding this comment

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

Please split PR into 3 job

@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Potabk Potabk changed the title [2/N] Enable shellcheck and pymarkdown for lint system [2/N] Enable shellcheck for lint system Jul 25, 2025
@Potabk Potabk changed the title [2/N] Enable shellcheck for lint system [2/N] Lint enhancements Jul 25, 2025
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Potabk added 6 commits August 8, 2025 10:48
@Potabk
format
f7373b1 
Signed-off-by: wangli <wangli858794774@gmail.com>
@Potabk
format
62e1ac1 
Signed-off-by: wangli <wangli858794774@gmail.com>
@Potabk
format
3ef4317 
Signed-off-by: wangli <wangli858794774@gmail.com>
@Potabk
format
d7a61cc 
Signed-off-by: wangli <wangli858794774@gmail.com>
@Potabk
format
d4be4e5 
Signed-off-by: wangli <wangli858794774@gmail.com>
@Potabk
fix
d9d6593 
Signed-off-by: wangli <wangli858794774@gmail.com>
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

1 similar comment
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yikun Yikun Yikun approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation merge-conflicts module:core module:ops module:quantization module:tests module:tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Potabk @Yikun