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

feat: add detekt Android CI workflow, migrate to ktlint plugin #1122

Merged
merged 8 commits into from
Jul 3, 2024

Conversation

jamesarich
Copy link
Contributor

@jamesarich jamesarich commented Jul 2, 2024

This adds the ci workflow job for detekt.
image

migrates ktlint functionality to detekt plugin

From there, @andrekir, you should be able to add the required check to the branch protection settings, if desired.

@jamesarich jamesarich closed this Jul 2, 2024
This commit resolves various detekt and ktlint issues in the codebase, including:

- Adjusting
 function and property naming conventions
- Addressing long parameter lists and method complexities
- Removing magic numbers and unused parameters
- Fixing new line and max line length issues
- Updating detekt and ktlint configurations in the CI workflow
@jamesarich jamesarich reopened this Jul 2, 2024
@jamesarich jamesarich marked this pull request as draft July 2, 2024 17:29
This commit adds detekt and ktlint to the CI pipeline to ensure code quality and consistency.

The new jobs run on Ubuntu and perform the following steps:

- Checkout the code
- Validate the Gradle wrapper
- Mock necessary files for CI
- Set up JDK 17
- Setup Gradle
- Run detekt or ktlint checks
- Upload build reports as artifacts
This commit removes the detekt check from the CI workflow and moves the ktlint check to the
 pull request workflow.
This change streamlines the CI process and ensures that code style is checked before merging.
@jamesarich
Copy link
Contributor Author

sorry for the churn - first time using github ci

Renamed build reports artifacts to detekt-reports and ktlint-reports for clarity.
@jamesarich jamesarich marked this pull request as ready for review July 2, 2024 17:48
@andrekir
Copy link
Member

andrekir commented Jul 2, 2024

can we have all ktlint/detekt files in their respective /config folders?

<?xml version="1.0" encoding="utf-8"?>
<baseline version="1.0">

<?xml version="1.0" encoding="utf-8"?>
<baseline version="1.0">

build:
maxIssues: 0

<?xml version="1.0" ?>
<SmellBaseline>

Configure Detekt and Ktlint with baseline files and move the Detekt configuration to the root level.
@jamesarich
Copy link
Contributor Author

Put the configs and baselines in the respective root /config folders and updated gradle configs to look for them there.

@jamesarich
Copy link
Contributor Author

actually - doing some reading and i may scrap ktlint entirely in favor of just using the ktlint formatting rules plugin available to detekt - moving this back to draft.

@jamesarich jamesarich marked this pull request as draft July 2, 2024 19:49
This commit migrates ktlint functionality to the detekt formatting plugin.

The ktlint Gradle plugin and related configurations have been removed. Instead, the detekt formatting plugin is now used to enforce code style and formatting.

This change simplifies the project's linting setup and consolidates code style enforcement under detekt.
@jamesarich jamesarich marked this pull request as ready for review July 2, 2024 20:11
@jamesarich
Copy link
Contributor Author

Ok, just using the ktlint detekt plugin now.

@jamesarich jamesarich changed the title feat: add detekt and ktlint checks to Android CI workflow. feat: add detekt Android CI workflow, migrate to ktlint plugin Jul 2, 2024
@andrekir andrekir merged commit 3dd0f8c into meshtastic:master Jul 3, 2024
5 checks passed
@jamesarich jamesarich deleted the github-actions-detekt-ktlint branch July 14, 2024 12:44
danwelch3 pushed a commit to danwelch3/meshtastic-android that referenced this pull request Oct 3, 2024
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.

2 participants