Skip to content

Conversation

oguzkocer
Copy link
Contributor

@oguzkocer oguzkocer commented May 13, 2024

Addresses #15.

  • Adds & configures the Detekt Gradle plugin.
  • Adds a CI step to run detektMain & detektTest tasks. I picked these over the detekt task because they include the new type resolution feature.
  • Excludes the generated UniFFI bindings wp_api.kt. I tried to do this in a way where we could exclude all source code under build/generated/source, but couldn't get that to work. I also tried using the detekt.yml config file to do the same, but I could only do it per rule and couldn't get the excludes attribute to work for this file.

@oguzkocer oguzkocer added this to the 0.1 milestone May 13, 2024
@oguzkocer oguzkocer force-pushed the add/detekt branch 3 times, most recently from c231879 to 9fde65c Compare May 13, 2024 19:05
@oguzkocer oguzkocer marked this pull request as ready for review May 13, 2024 20:20
@oguzkocer oguzkocer enabled auto-merge (squash) May 13, 2024 20:20
Copy link
Member

@wzieba wzieba 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

Comment on lines +93 to +100
echo "--- :rust: Installing Rust"
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -v -y
source "$$HOME/.cargo/env"
echo "--- :package: Installing Rust Toolchains"
make setup-rust
make setup-rust-android-targets
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this could be a separate step? If I was debugging it, I wouldn't expect Detekt step to install Rust dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is here because it's necessary for the project to build at the moment. I agree that it's not ideal and I'll see if I can get rid of that requirement when I can - but for now, having these extra steps is not a big deal.

@oguzkocer oguzkocer merged commit d593a2f into trunk May 14, 2024
@oguzkocer oguzkocer deleted the add/detekt branch May 14, 2024 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants