JeffDarchuk / SitecoreSidekick

Framework for angularjs based microservice architecture operations.
MIT License
48 stars 33 forks source link

Discussion - ContentItemInstaller #74

Closed markgibbons25 closed 3 years ago

markgibbons25 commented 3 years ago

A couple things:

https://github.com/JeffDarchuk/SitecoreSidekick/blob/44c77def2f74ca41e72b3107389e3f0a61d48a99/ContentMigrator/Core/ContentItemInstaller.cs#L320

Should this not have a _logger.BeginEvent(remoteData, LogStatus.Skipped, GetSrc(_sitecore.GetIconSrc(localData)), false);?

2.

https://github.com/JeffDarchuk/SitecoreSidekick/blob/44c77def2f74ca41e72b3107389e3f0a61d48a99/ContentMigrator/Core/ContentItemInstaller.cs#L299

When is args.Overwrite going to be set? Is this some old feature?

3.

https://github.com/JeffDarchuk/SitecoreSidekick/blob/44c77def2f74ca41e72b3107389e3f0a61d48a99/ContentMigrator/Core/ContentItemInstaller.cs#L306

Looks like we can safely use _comparer.FastCompare for a minor performance improvement here.

JeffDarchuk commented 3 years ago
  1. This line doesn't actually indicate a skip per se, but rather an error when adding an ancestor of the item and because of that it should be skipped and not processed.
  2. Overwrite is a toggle that's on by default, you can uncheck this in the interface. Doing so will result in sidekick not removing local content. This can be useful if there's meaningful test content that QA people don't want overwritten but they DO want NEW content.
  3. It's been a really long time, but there is a reason i didn't use FastCompare here. I don't recall exactly why however since that was about 5 years ago, might be worth another look