[Bug 43598] Add high-level API for Proto.I.Addressing interface

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=43598

Andre Moreira Magalhaes <andrunko at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #11 from Andre Moreira Magalhaes <andrunko at gmail.com> 2011-12-19 08:05:52 PST ---
(In reply to comment #10)
> (In reply to comment #9)
> > Branch updated with the following tests:
> > 1) all info in .manager file
> > 2) no .manager file, but some immutable properties present, though not all
> > 3) all immutable props present
> > 4) no .manager file, no immutable properties present
> > 
> 
> The tests look nice, except for one thing:
> 
> +    static int introspectionCalled;
> 
> Why are these statics? Apart from the reset thing (which should be a method on
> the CM helper object, or perhaps just creating a new CMHelper object in the
> beginning of each test case as needed, and deleting it in cleanup() IMO), you
> seem to access them through an instance pointer. This looks a bit messy to me,
> could you please clean it up slightly?
> 
> Having them as non-statics would make the test more flexible wrt. future
> changes, especially ones where we have multiple adaptors of a single kind (for
> multiple protocols) instantiated at a time, so we should do it unless there is
> some really big benefit for the current usage in making them statics.
Done.

> > I also fixed an issue with the introspection code.
> 
> Ah, it didn't properly extract the interfaces from received properties before
> checking for them to decide what to introspect next? Very good fix - this is
> why we unit-test our code! (cf.
> http://blogs.msdn.com/cfs-filesystemfile.ashx/__key/communityserver-blogs-components-weblogfiles/00-00-01-32-02-metablogapi/7317.image_5F00_0F65063B.png)
lol :)

> I assume the test coverage is now very much in the green zone, with all these
> important tests?
Yes, the new Protocol bits are basically fully tested and ConnectionManager
coverage raised to 79.1% now.

> Overall, a very good job. After perhaps cleaning up the introspection counters
> a bit, please merge, along with the conn.i.addressing branch you based on top
> of this.
Thanks, very good review also.

Changes merged upstream, it will be in tp-qt 0.9.0

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are the assignee for the bug.