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

refactor: clippy fixes and working doctests #834

Merged
merged 8 commits into from
Nov 6, 2024

Conversation

Miitto
Copy link
Contributor

@Miitto Miitto commented Nov 5, 2024

Linting

Fixes most lint errors provided by clippy

Unused variables / functions have not been fixed

Clippy detects needless_return on both main functions, which I believe is caused by #[tokio::main], so these have been supressed.

Doctests

The following are no longer run:

  • wm\src\containers\commands\flatten_child_split_containers.rs:11 : flatten_child_split_containers has been ignored
  • wm\src\common\platform\platform.rs:340: parse_command has been set to no_run as it requires files to exist on the test machine.

The following have been fixed so that they can be compiled and ran:

  • wm\src\common\tiling_direction.rs:19: inverse
  • wm\src\common\tiling_direction.rs:35: from_direction
  • wm\src\common\tiling_direction.rs:54: from_str
  • wm\src\common\direction.rs:19 : inverse
  • wm\src\common\direction.rs:40 : from_str
  • wm\src\common\length_value.rs:52: from_str

packages/wm/src/workspaces/commands/focus_workspace.rs Outdated Show resolved Hide resolved
packages/wm/src/user_config.rs Outdated Show resolved Hide resolved
packages/wm/src/common/platform/platform.rs Outdated Show resolved Hide resolved
packages/watcher/src/main.rs Outdated Show resolved Hide resolved
packages/wm/src/main.rs Outdated Show resolved Hide resolved
@lars-berger lars-berger changed the title Feat: fix Lint errors and doctests refactor: clippy fixes and working doctests Nov 5, 2024
@lars-berger
Copy link
Member

This is super nice!! Been meaning to add clippy linting for ages 🙌

Would you happen to know what the recommended command would be to to add linting to CI?

@Miitto
Copy link
Contributor Author

Miitto commented Nov 5, 2024

Not sure how to add it to CI, but it can be manually run via cargo clippy.

Can be added to the tool chain via rustup component add clippy if it is not already present.

Docs here

on: push
name: Clippy check

# Make sure CI fails on all warnings, including Clippy lints
env:
  RUSTFLAGS: "-Dwarnings"

jobs:
  clippy_check:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - name: Run Clippy
        run: cargo clippy --all-targets --all-features

Example taken from here

Additional info from the clippy GH

@lars-berger lars-berger merged commit bc7839a into glzr-io:main Nov 6, 2024
@lars-berger
Copy link
Member

Merged 👍

Will look into getting rid of those unused var warnings - would be super nice to enforce these lints for PR's. Ty for the info on adding it to CI!

Copy link

🎉 This PR is included in version 3.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Copy link

🎉 This PR is included in version 3.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@Miitto Miitto deleted the feat-lint-fixes branch January 22, 2025 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants