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.

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

Move AppDaysUsedRepository to browser-api so it can be accessed from autofill #4474

Merged
merged 1 commit into from May 9, 2024
Merged

Move AppDaysUsedRepository to browser-api so it can be accessed from autofill #4474

merged 1 commit into from May 9, 2024

Conversation

CDRussell
Copy link
Member

@CDRussell CDRussell commented Apr 26, 2024 •
edited

Task/Issue URL: https://app.asana.com/0/0/1207179707350731/f

Description

Moves a class from app module to browser-api so it can be used in autofill survey.

Steps to test this PR

  • QA-optional

@CDRussell Graphite App
Copy link
Member Author

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

Join @CDRussell and the rest of your teammates on Graphite Graphite

This was referenced Apr 26, 2024
Merged
Merged
@CDRussell CDRussell force-pushed the feature/craig/autofill_survey_groundwork branch from f934c9f to 1ce80b3 Compare April 26, 2024 14:17
@CDRussell CDRussell marked this pull request as ready for review April 26, 2024 15:21
cmonfortep
cmonfortep approved these changes May 2, 2024
View reviewed changes
Copy link
Contributor

@cmonfortep cmonfortep left a comment

Choose a reason for hiding this comment

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

LGTM

browser-api/src/main/java/com/duckduckgo/app/usage/app/AppDaysUsedRepository.kt
/**
* Repository for storing and retrieving the number of days the app has been used
*/
interface AppDaysUsedRepository {
Copy link
Contributor

Choose a reason for hiding this comment

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

QQ: Are we planning to revert this once survey completed?

sgtm now to unblock and release the autofill survey. However, if final, we should discuss the api change to validate what makes sense to expose, how, where, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wasn't planning to revert, as the changes seem generally useful.

we should discuss the api change

that was why it was added to the API proposal a few weeks back.

sgtm now to unblock and release the autofill survey

Ok, I will carry on with this as planned and can discuss if there should be additional changes to the API made afterwards.

@CDRussell CDRussell force-pushed the feature/craig/autofill_survey_groundwork branch from 1ce80b3 to f0eb63f Compare May 3, 2024 14:04
@CDRussell CDRussell force-pushed the feature/craig/autofill_survey_groundwork branch from f0eb63f to 65177a1 Compare May 6, 2024 12:15
@CDRussell Graphite App
Copy link
Member Author

CDRussell commented May 9, 2024 •
edited

Merge activity

@CDRussell CDRussell merged commit d36daab into develop May 9, 2024
5 checks passed
@CDRussell CDRussell deleted the feature/craig/autofill_survey_groundwork branch May 9, 2024 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmonfortep cmonfortep cmonfortep approved these changes

Assignees

@cmonfortep cmonfortep

Labels
QA-Optional
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@CDRussell @cmonfortep