Skip to content

ADS: Move all ADS modules under the same folder #6975

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Nov 5, 2025
Merged

ADS: Move all ADS modules under the same folder #6975

merged 11 commits into from
Nov 5, 2025

Conversation

@malmstein
Copy link
Contributor

@malmstein malmstein commented Oct 22, 2025 •
edited by mikescamell
Loading

Task/Issue URL: https://app.asana.com/1/137249556945/project/1202552961248957/task/1211716560268930?focus=true

Description

Move all ADS related modules under the same folder

Steps to test this PR

Smoke tests

  • Open the app installing the Play flavour
  • Play around to verify nothing is broken

Play Flavour

  • Open the app installing the Play flavour
  • Navigate to settings
  • Verify that the ADS Preview feature is not visible

Internal Flavour

  • Open the app installing the Internal flavour
  • Navigate to settings
  • Verify that the ADS Preview feature is visible

@malmstein malmstein mentioned this pull request Oct 22, 2025
Merged
6 tasks
@malmstein Graphite App
Copy link
Contributor Author

malmstein commented Oct 22, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@malmstein malmstein marked this pull request as ready for review October 22, 2025 09:10
@cmonfortep
Copy link
Contributor

cmonfortep commented Oct 22, 2025 •
edited
Loading

@malmstein #6974 mitigates the issue, and that's enough for the hotfix.
While doing this task, which it's just something I was going to ask as well, can we also move into the internal module the Activity for internal settings, and any other component that should be internal only?

@malmstein
Copy link
Contributor Author

malmstein commented Oct 22, 2025

Yes @cmonfortep, makes sense!

Base automatically changed from feature/david/10-22-ads_add_theme_preview_as_internal to develop October 22, 2025 09:19
@malmstein malmstein changed the base branch from develop to graphite-base/6975 October 22, 2025 14:53
@malmstein malmstein changed the base branch from graphite-base/6975 to develop October 22, 2025 14:53
@malmstein malmstein changed the base branch from develop to graphite-base/6975 October 22, 2025 14:59
@malmstein malmstein force-pushed the feature/david/10-22-ads_move_all_ads_modules_under_the_same_folder branch from 68fda71 to 5e5862a Compare October 22, 2025 14:59
@malmstein malmstein changed the base branch from graphite-base/6975 to develop October 22, 2025 14:59
@github-actions
Copy link
Contributor

github-actions bot commented Oct 22, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 22, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@malmstein malmstein force-pushed the feature/david/10-22-ads_move_all_ads_modules_under_the_same_folder branch from 5e5862a to 11177a8 Compare October 23, 2025 09:47
@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@malmstein malmstein force-pushed the feature/david/10-22-ads_move_all_ads_modules_under_the_same_folder branch from 2f3d0de to dd2792e Compare October 23, 2025 13:17
@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@malmstein malmstein force-pushed the feature/david/10-22-ads_move_all_ads_modules_under_the_same_folder branch from dd2792e to 2376bb6 Compare November 3, 2025 15:30
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@malmstein malmstein force-pushed the feature/david/10-22-ads_move_all_ads_modules_under_the_same_folder branch from 2376bb6 to 8719717 Compare November 3, 2025 17:03
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@malmstein malmstein force-pushed the feature/david/10-22-ads_move_all_ads_modules_under_the_same_folder branch from f9fd848 to 4f8a1e0 Compare November 3, 2025 21:41
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

❌ Design System Maestro tests failed. Please check the workflow run for details.

@malmstein malmstein force-pushed the feature/david/10-22-ads_move_all_ads_modules_under_the_same_folder branch from 88b6369 to ef853d8 Compare November 4, 2025 15:54
@malmstein
Copy link
Contributor Author

malmstein commented Nov 4, 2025

@mikescamell this is finally ready for review again

mikescamell
mikescamell reviewed Nov 5, 2025
View reviewed changes
Copy link
Contributor

@mikescamell mikescamell left a comment

Choose a reason for hiding this comment

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

We've named the top level package android-design-system but within we've named modules just design-system, is this intentional?

Should we name the modules android-design-system-internal etc. to match other features?

mikescamell
mikescamell approved these changes Nov 5, 2025
View reviewed changes
Copy link
Contributor

@mikescamell mikescamell left a comment

Choose a reason for hiding this comment

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

LGTM

I can't see the ADS shortcut on my Pixel 4a, have we reached the max limit of shortcuts with Duck.ai? Anyway I don't think it should stop us merging.

@malmstein malmstein merged commit 7efc550 into develop Nov 5, 2025
22 checks passed
@malmstein malmstein deleted the feature/david/10-22-ads_move_all_ads_modules_under_the_same_folder branch November 5, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mikescamell mikescamell mikescamell approved these changes

Assignees

@mikescamell mikescamell

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@malmstein @cmonfortep @mikescamell