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

chore: make ruff linter checks cover tests and apply pycodestyle rules #2802

Closed
wants to merge 3 commits into from

Conversation

bowenliang123
Copy link
Contributor

@bowenliang123 bowenliang123 commented Mar 12, 2024

Description

  • apply pycodestyle's E rules and W rules: https://docs.astral.sh/ruff/rules/#pycodestyle-e-w

    • E rules help to prevent mixed misuse in indention and bad practices in codes
    • W rules help to ensure a new line at the end of the file, remove tailing whitespace and etc.
  • apply ruff linter checks to previously ignored python files in tests, __init__.py and app.py, allowing unused imports for preparation of mocking or handling

    • e.g., the unused imports in app.py are properly preserved.
    from events import event_handlers
    from models import account, dataset, model, source, task, tool, tools, web
    

Type of Change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update, included: Dify Document

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • TODO

Suggested Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
  • optional I have made corresponding changes to the documentation
  • optional I have added tests that prove my fix is effective or that my feature works
  • optional New and existing unit tests pass locally with my changes

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 12, 2024
Comment on lines 26 to 50
[tool.ruff.lint.per-file-ignores]
"app.py" = [
"F401", # unused-import
"F811", # redefined-while-unused
]
"__init__.py" = [
"F401", # unused-import
"F811", # redefined-while-unused
]
"tests/*" = [
"F401", # unused-import
"F811", # redefined-while-unused
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allowing unused imports in these scripts and tests.

@bowenliang123 bowenliang123 changed the title chore: apply ruff linter checks on previously ignored tests and scripts with allowing unused imports for mocking and handling chore: apply ruff linter checks on previously ignored tests and scripts allowing unused imports for mocking and handling Mar 12, 2024
@takatost
Copy link
Collaborator

The workflow version codes has a major refactoring, which may cause a lot of conflicts. We will proceed with this PR after the workflow version is released.

@bowenliang123
Copy link
Contributor Author

No problem. I will keep this updated when the mater branch merges the workflow feature. It's easy to rebase the rules on to new commits. Thanks for reviewing. @takatost @crazywoola

@bowenliang123 bowenliang123 changed the title chore: apply ruff linter checks on previously ignored tests and scripts allowing unused imports for mocking and handling chore: make ruff linter checks cover tests and apply pycodestyle rules Mar 17, 2024
@bowenliang123 bowenliang123 force-pushed the ruff-plus branch 5 times, most recently from 431e1b0 to 8fc3a36 Compare April 9, 2024 13:32
@bowenliang123
Copy link
Contributor Author

Hi @takatost , all the rule configs and fixes are now up to date and ready for review. It could be a good timing considering it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants