Hi Jack,
Comments inline.
Thank you,
Clay
On Thu, 16 Oct 2008, Jack Schwartz wrote:
>
> Hi Clay.
>
> My comments are based on webrev_final_rc4:
>
> publish_manifest.py:
> --------------------------
>
> General comment: While indentation is much improved, indentation for
> continuation lines of statements is still erratic. Continuation lines
> should be indented by 4 spaces, whether subsequent lines contain method
> arguments or other things. Check out lines 571, 572, 560, 561, others...
>
> set/getManifestPath:
> 614: Why not just check if (self._manifest != None)? You know it'll be a
> string if set because line 624 is the only place which sets it.
> Otherwise, these methods look fine.
Fixed with != None
> set/getService:
> Same question for self._service on line 523 as for self._manifest on 614.
Ditto
> AI_database.py
> --------------------
>
> set/getResponse():
>
> 108: typo: "event flag"
I'm no longer missing that t...
> Just curious: where does self._e get cleared, or are DBrequest objects
> short lived and clearing is unnecessary?
Short lived, single use objects
> DB functions look fine to me.
>
> Thanks,
> Jack
>
>
>
>
> Clay Baenziger wrote:
> > Hi Dave and Jack,
> > You both raised concerns with the response(), manifestPath(),
> > service() and database() functions in AI_database, and publish_manifest
> > which did not have explicit set and accessor functions. I've have
> > corrected that in:
> > http://cr.opensolaris.org/~clayb/AI_server/webrev_final_rc3/.
> >
> > Thank you,
> > Clay
> >
>
>
