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

Disable reset sync progress marker study; fixes #569. Uplift to production #577

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

AlexeyBarabash
Copy link
Contributor

This is uplift of #570

Steps to verify the Griffin change is applied. This requires local modified build and hard to test with common available installers, because requires profile folder manipulation and it is not 100% reliable.

  1. Make a local build with patch
--- a/components/sync/engine/brave_model_type_worker.cc
+++ b/components/sync/engine/brave_model_type_worker.cc
@@ -59,8 +59,10 @@ void BraveModelTypeWorker::OnCommitResponse(
                                     error_response_list);
 
   if (!base::FeatureList::IsEnabled(features::kBraveSyncResetProgressMarker)) {
+LOG(ERROR) << "[BraveSync] " << __func__ << " DISABLED kBraveSyncResetProgressMarker";
     return;
   }
+LOG(ERROR) << "[BraveSync] " << __func__ << " ENABLED kBraveSyncResetProgressMarker";
  1. Launch own-built binary with args
./brave --user-data-dir=<your test data dir>   --enable-logging=stderr --v=0 \
         --variations-server-url=https://variations.bravesoftware.com/seed \
--variations-insecure-server-url=https://variations.bravesoftware.com/seed \
--vmodule=study_filtering=1 \
--fake-variations-channel=canary
  1. Enable sync if not enabled and add some bookmark
  2. Ensure you can see in log
brave_model_type_worker.cc(62)] [BraveSync] OnCommitResponse DISABLED kBraveSyncResetProgressMarker

@AlexeyBarabash AlexeyBarabash requested a review from a team as a code owner March 31, 2023 12:09
@AlexeyBarabash AlexeyBarabash force-pushed the disable-sync-reset-progress-prod branch 2 times, most recently from 266207e to 7258901 Compare April 6, 2023 20:09
@AlexeyBarabash AlexeyBarabash force-pushed the disable-sync-reset-progress-prod branch from 7258901 to 6f9962c Compare April 14, 2023 08:26
Copy link
Member

@kjozwiak kjozwiak left a comment

Choose a reason for hiding this comment

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

Uplift into production approved after deliberating with @brave/uplift-approvers. QA has verified the PR on main (staging) as per #570 (comment).

@kjozwiak kjozwiak merged commit 057e0a8 into production Apr 14, 2023
@kjozwiak kjozwiak deleted the disable-sync-reset-progress-prod branch April 14, 2023 11:36
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