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

Remove Showkase processor not found warning from Danger #3148

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

jmartinesp
Copy link
Member

@jmartinesp jmartinesp commented Jul 5, 2024

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other : CI checks cleanup

Content

Remove Danger check to ensure Showkase is enabled in modules with previews. Instead try to check if the package of the preview is in the list of packages to scan in ComposablePreviewProvider.

Motivation and context

We don't use Showkase anymore for this.

Tests

The CI failed before when the new rule was broken: #3148 (comment)

Checklist

  • Changes have been tested on an Android device or Android emulator with API 23
  • UI change has been tested on both light and dark themes
  • Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
  • Pull request is based on the develop branch
  • Pull request title will be used in the release note, it clearly define what will change for the user
  • Pull request includes screenshots or videos if containing UI changes
  • Pull request includes a sign off
  • You've made a self review of your PR

@jmartinesp jmartinesp force-pushed the misc/jme/remove-showkase-warning-from-danger branch 2 times, most recently from dc2dc9e to 482f166 Compare July 5, 2024 11:20
Copy link
Contributor

github-actions bot commented Jul 5, 2024

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/QYt7GR

Copy link

codecov bot commented Jul 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.03%. Comparing base (53ff503) to head (a6f9762).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3148   +/-   ##
========================================
  Coverage    76.03%   76.03%           
========================================
  Files         1635     1635           
  Lines        38608    38608           
  Branches      7464     7464           
========================================
  Hits         29354    29354           
  Misses        5366     5366           
  Partials      3888     3888           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jmartinesp jmartinesp added the PR-Build For changes related to build, tools, CI/CD label Jul 5, 2024
@jmartinesp jmartinesp force-pushed the misc/jme/remove-showkase-warning-from-danger branch from aaad970 to 373ca5a Compare July 5, 2024 11:51
@jmartinesp
Copy link
Member Author

image

🎉

Instead create a new rule to check if the package name is included in `ComposablePreviewProvider`.
@jmartinesp jmartinesp force-pushed the misc/jme/remove-showkase-warning-from-danger branch from 373ca5a to a6f9762 Compare July 5, 2024 11:55
@jmartinesp jmartinesp marked this pull request as ready for review July 5, 2024 11:57
@jmartinesp jmartinesp requested a review from a team as a code owner July 5, 2024 11:57
@jmartinesp jmartinesp requested review from bmarty and removed request for a team July 5, 2024 11:57
Copy link

sonarcloud bot commented Jul 5, 2024

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jmartinesp jmartinesp merged commit 1f69722 into develop Jul 5, 2024
26 checks passed
@jmartinesp jmartinesp deleted the misc/jme/remove-showkase-warning-from-danger branch July 5, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Build For changes related to build, tools, CI/CD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants