Specifically:
wifi_sync -> sync_wifi
syncable_prefs -> sync_preferences
Namespaces should be updated to match as well. Colin, do you think password_manager/sync should become sync_passwords, or should it continue living inside the password_manager component?
I'm fine with #1. If you feel like making it a component is an overkill, we can also just merge it with password_manager/core; there is unlikely to be other implementation of the functionality than sync. When we added the sync layer to password_manager, we were aiming at fine-grained dependency separation inside the component, which might not be what layers were supposed to be used for?
Also Cc-ing engedy@, who might have interesting input, because he did the code reviews when we created the sync layer.
I like #1 as well. It looks like it would trivial to move out password_manager/sync into sync_passwords without ugly cross-component dependencies. Doing so would create a component that is small, but would be consistent with other sync_* components.
Actually, thinking on this further, I'm less convinced the passwords sync code should be its own component. sync_bookmarks and sync_sessions exist because that code is owned by the sync team. sync_preferences makes sense for layering because sync depends on prefs. I'm not exactly sure why sync_wifi is a thing outside the wifi component, and I'm not sure why the passwords sync code shouldn't be inside password_manager/core/.
I don't think we want to set a precedent for types to create a separate component for their sync code without need. There are many other types with sync code inside their component and I think it makes more sense to do it that way: the namespaces can match, and the new version of sync is going to be more tightly coupled to the model type code anyways.
Thoughts on this? Am I missing something?
+Nicolas (for context) and Sky (made sync_sessions in http://crrev.com/1387253004). Colin, you made sync_bookmarks (in http://crrev.com/1395563002) and said:
"The decision to create a standalone component rather than folding these files
into the bookmarks component is that it is undesirable to introduce a
sync-related dependency in the core bookmark component."
Essentially, I think it's because there were files sync had always owned before related to these types that needed to be moved into components/ and the existing sessions/ and bookmarks/ owners didn't want them, or we just wanted to keep ownership of those pieces.
There's no real reason the model type components shouldn't depend on sync AFAIK.
I see, thanks for the reminders. My concern is that I think we should be consistent. I had mistakenly remembered that we had previously discussed this and explicitly decided to move all components' sync-related code into sync_<foo>, but it looks like many components depend directly on //components/sync from their code code, and it's actually the sync_<foo> components that are the outliers. So merging passwords sync code into password_manager/core SGTM given that it doesn't change the pre-existing situation at all. I think that sync_sessions and sync_bookmarks might be their own components simply because the OWNERS didn't want that code in their components, which is kind of a sad reason to create this inconsistency.
True. sync_preferences actually needs to be its own component for layering reasons though. sync_wifi is probably the one that maybe shouldn't be its own component. I'm going to close this bug though, as it's fulfilled its original purpose. If there are further cleanups related to this you still want done, feel free to file separate bugs against the relevant owners.
Comment 1 by blundell@chromium.org
, Oct 25 2016