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

SwiftLint plugin #393

Merged
merged 25 commits into from
Dec 18, 2023
Merged

SwiftLint plugin #393

merged 25 commits into from
Dec 18, 2023

Conversation

mallexxx
Copy link
Collaborator

@mallexxx mallexxx commented Jun 28, 2023

Please review the release process for BrowserServicesKit here.

Required:

Task/Issue URL: https://app.asana.com/0/276630244458377/1204377796839424/f
iOS PR: duckduckgo/iOS#2233
macOS PR: duckduckgo/macos-browser#1318
What kind of version bump will this require?: Patch

Optional:

Tech Design URL:
CC: @samsymons

Description:

  • SwiftLint added as build plugin

  • Implemented swiftlint results caching based on source files modification date

  • ❗️ See fix linter issues #592 for fixed linter issues

Steps to test this PR:
1.
1.

OS Testing:

  • iOS
  • macOS

Internal references:

Software Engineering Expectations
Technical Design Template

@samsymons samsymons self-assigned this Jul 28, 2023
Comment on lines 11 to 12
- force_cast
- force_try
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@samsymons
Copy link
Contributor

This is really cool - @mallexxx if you have some time this week, are you able to pull changes from main into this branch so we can test it against the latest code?

I'm also curious, have you also tried the official SwiftLint package plugin and did you find it insufficient?

@mallexxx
Copy link
Collaborator Author

@samsymons, I‘ve merged the upstream. Of course I‘ve checked the official plugin, in fact current implementation is based on it (as it‘s pretty straightforward) - the difference is the official plugin runs the check against all the files and my implementation caches results to improve build speed

@mallexxx
Copy link
Collaborator Author

the main problem now is to adjust the plugin to run on CI and export the linter check results

@mallexxx mallexxx assigned mallexxx and unassigned samsymons Dec 5, 2023
@mallexxx mallexxx changed the title [WIP] SwiftLint plugin SwiftLint plugin Dec 6, 2023
@mallexxx mallexxx requested a review from samsymons December 6, 2023 13:44
@mallexxx mallexxx assigned samsymons and unassigned mallexxx Dec 7, 2023
Copy link
Contributor

@samsymons samsymons left a comment

Choose a reason for hiding this comment

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

The plugin worked well for me in client PRs, nice work! And being able to get SwiftLint annotations inline when working on BSK directly is great. 👏

@mallexxx mallexxx merged commit d07e18b into main Dec 18, 2023
5 checks passed
@mallexxx mallexxx deleted the alex/swiftlint-plugin branch December 18, 2023 07:23
mallexxx added a commit to duckduckgo/macos-browser that referenced this pull request Dec 18, 2023
samsymons added a commit that referenced this pull request Dec 21, 2023
* main: (39 commits)
  Fix privacy config fetch in debug mode (#606)
  Expose Internal User managing from Config (#610)
  Add Sync feature flags (#607)
  Fix Networking import into TestUtils (#609)
  Add Sync Success Rate pixel (#605)
  Add new logger (#604)
  Prevent VPN server list persistence failures (#603)
  SwiftLint plugin (#393)
  Update autofill to 10.0.2 (#599)
  Remove the reconnect/disconnect logic from the connection tester
  Fix an IPv6 regression. (#598)
  Quality metrics for Sync (#597)
  Report NetP connection attempts, tunnel failures, and latency (#584)
  Implement deleteAccount Sync endpoint (#596)
  Ensure that LinkPresentation framework is called on main thread (#595)
  No longer excluding the 10.0.0.0/8 range (#594)
  Update autofill to 10.0.1 (#591)
  Implement deleteAccount Sync endpoint (#596)
  Ensure that LinkPresentation framework is called on main thread (#595)
  No longer excluding the 10.0.0.0/8 range (#594)
  ...
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