Skip to content

[Brave News]: Add sync support#37144

Open
fallaciousreasoning wants to merge 1 commit into
masterfrom
bn-sync
Open

[Brave News]: Add sync support#37144
fallaciousreasoning wants to merge 1 commit into
masterfrom
bn-sync

Conversation

@fallaciousreasoning

Copy link
Copy Markdown
Contributor

Resolves brave/brave-browser#23054
Closes #37013
Closes brave/go-sync#429

I had a play around with more complicated sync strategies here but merging gets a lot more complicated unless we add timestamps to individual entires. I think for the most part this should be a pretty massive improvement.

One case where this won't work properly is if you make edits on two different devices, the first edit will win.

@fallaciousreasoning

Copy link
Copy Markdown
Contributor Author

Note for reviewers: This is my first syncable pref, so if something looks wrong it probably is.

@github-actions

Copy link
Copy Markdown
Contributor

📋 Code Owners Summary

4 file(s) changed, 1 with assigned owners

1 team(s) affected: @brave/sync-reviewers


Owners and Their Files

@brave/sync-reviewers — 1 file(s)

@brave-builds

Copy link
Copy Markdown
Collaborator

Warning

You have got a presubmit warning. Please address it if possible.

Discovered possible removal of preference registrations.

Please make sure to properly deprecate preferences by clearing their
value for a couple of milestones before finally removing the code.
Otherwise data may stay in the preferences files forever. See
Migrate*Prefs() in chrome/browser/prefs/browser_prefs.cc and
chrome/browser/prefs/README.md for examples.
This may be a false positive warning (e.g. if you move preference
registrations to a different place or if the preference was only
written to by the policy stack).

Items:

components/brave_news/common/pref_names.cc: -  registry->RegisterBooleanPref(kShouldShowToolbarButton, true);
components/brave_news/common/pref_names.cc: -  registry->RegisterBooleanPref(kNewTabPageShowToday,
components/brave_news/common/pref_names.cc: -  registry->RegisterBooleanPref(kBraveNewsOptedIn, false);
components/brave_news/common/pref_names.cc: -  registry->RegisterDictionaryPref(kBraveNewsSources);
components/brave_news/common/pref_names.cc: -  registry->RegisterDictionaryPref(kBraveNewsChannels);
components/brave_news/common/pref_names.cc: -  registry->RegisterDictionaryPref(kBraveNewsDirectFeeds);
components/brave_news/common/pref_names.cc: -  registry->RegisterBooleanPref(kBraveNewsOpenArticlesInNewTab, true);

@@ -1,5 +1,6 @@
include_rules = [
"+components/prefs",
"+components/pref_registry",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: pref_registry_syncable.h is only included in pref_names.cc, so this allowlist entry belongs in specific_include_rules keyed to pref_names\.cc rather than the broad include_rules. See BS-030.

@aguscruiz

Copy link
Copy Markdown
Collaborator

HUGEEEEEE

{"brave.today.userfeeds",
{brave_syncable_prefs_ids::kBraveNewsDirectFeeds, syncer::PREFERENCES,
sync_preferences::PrefSensitivity::kNone,
sync_preferences::MergeBehavior::kNone}},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm a bit worried about this on initial sync

@github-actions

Copy link
Copy Markdown
Contributor

Chromium major version is behind target branch (149.0.7827.59 vs 150.0.7871.13). Please rebase.

@github-actions github-actions Bot added the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Include custom Brave News sources and/or setup in Sync

4 participants