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

Fix broken lint and add it to GH Actions #511

Merged
merged 2 commits into from
Nov 29, 2020
Merged

Fix broken lint and add it to GH Actions #511

merged 2 commits into from
Nov 29, 2020

Conversation

cortinico
Copy link
Member

📄 Context

Lint is currently broken on develop. I'm fixing it and I've added a job on GH Actions to run it on pre-merge. If you happen to run gw check to run all the verifications, it will fail.

📝 Changes

  • Disabled AcceptsUserCertificates as we intentionally want to allow user CAs
  • Fixed error on android:networkSecurityConfig

⏱️ Next steps

I believe we should update the pre-commit hook to run gw check

@cortinico cortinico added the infra Issue or pull request relate to the library infrastructure (CI, Release mgmt, etc.) label Nov 28, 2020
Copy link
Collaborator

@vbuberen vbuberen left a comment

Choose a reason for hiding this comment

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

I am not sure I understand what is gw check?

@cortinico
Copy link
Member Author

I am not sure I understand what is gw check?

gw is a shorthand for ./gradlew (a bash alias).
check is a lifecycle task to run all the verifications of a project (such as Detekt, KtLint, tests, etc.)

@vbuberen
Copy link
Collaborator

Understood. Wanted to be sure.

Copy link
Collaborator

@vbuberen vbuberen left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

<!-- Default screen margins, per the Android Design guidelines. -->
<dimen name="norm_grid_size">8dp</dimen>
<dimen name="norm_grid_size" tools:ignore="UnusedResources">8dp</dimen>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why it is ignored?
Shouldn't be just removed if it is really unused?
Or maybe it is preserved for backwards compatibility?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why it is ignored?

It's ignored as it's our base grid size. All the others size are derived from this. I'd rather keep it otherwise doub_grid_size would make little sense

@cortinico cortinico merged commit 214deda into develop Nov 29, 2020
@cortinico cortinico deleted the nc/fix-lint branch November 29, 2020 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Issue or pull request relate to the library infrastructure (CI, Release mgmt, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants