Do

Merge lp://qastaging/~cszikszoy/do/newdo-service-loading into lp://qastaging/do/rewrite

Proposed by Chris S.
Status: Work in progress
Proposed branch: lp://qastaging/~cszikszoy/do/newdo-service-loading
Merge into: lp://qastaging/do/rewrite
Diff against target: 354 lines (+225/-32)
7 files modified
src/Backends/DBus/DBus.csproj (+3/-0)
src/Core/Base/Base.csproj (+2/-0)
src/Core/Base/src/Addins.cs (+130/-0)
src/Core/Base/src/ServiceManager.cs (+73/-31)
src/Core/Base/src/UniverseManager.cs (+1/-1)
src/Core/Services/Resources/Do.Services.addin.xml (+9/-0)
src/Core/Services/Services.csproj (+7/-0)
To merge this branch: bzr merge lp://qastaging/~cszikszoy/do/newdo-service-loading
Reviewer Review Type Date Requested Status
Alex Launi Needs Fixing
Review via email: mp+33493@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
Alex Launi (alexlauni) wrote :

1.) Please comment all classes to say overall what the class is for. Give a little design intro as to the reason for the class. If you think it's abundantly obvious make it short, but have a little something
1a.) What's the point of the Addins class? What is in there that should not just be in ServiceManager? If you think that the Actual Mono.Addins stuff should be its own class (which I think I agree with) then try and take the AddinManager calls out of Services and fully abstract it out.
2.)
+ try {
+ AddinManager.Initialize (pluginsPath);
+ } catch (Exception e) {
+ AddinManager.Registry.Rebuild (monitor);
+ AddinManager.Shutdown ();
+ AddinManager.Initialize (pluginsPath);
+ }
+
+ AddinManager.Registry.Update (monitor);

Why wouldn't the 2nd call to AddinManager throw the same exception the first one did? Why do you Rebuild, shutdown, and then initialize? Is it necessary to Update after the Rebuild, Shutdown, Init cycle? The Update seems needless. If there's a good reason for this please put it into a comment so that this is clearer.
3.) Why does the DBus csproj need a Mono.Addins reference?
4.) Use Hyena.Log class for logging.

review: Needs Fixing

Unmerged revisions

15. By Chris S.

lay the groundwork for loading of services via mono.addins

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: