Comment 1 by Thomas Keller, Jan 10, 2011
Labels:
Milestone:1.0
Status: Accepted
Owner: tommyd
Status: Accepted
Owner: tommyd
Comment 2 by Thomas Keller, Jan 11, 2011
Implemented in revision b5eabe26d8db8d24a28251912e7a6e38a01a1f64 - lets do the code review this time in ticket form, what do you think?
Status:
Fixed
Comment 3 by Thomas Keller, Jan 12, 2011
Actually, please base your review on revision cb6687e46fc6d115f51e888bf6c8b655fa524036 - I was a bit unconcentrated...
Comment 4 by Stephen Leake, Jan 12, 2011
code is excellent (compiling now on MinGW for tests). manual needs a little more work; reference get_default_database_glob in commands that use it (setup, checkout, list workspaces, list databases, --db, any others?), mention that setup, checkout register databases (any other commands do this?) should we mention the new Lua hook in NEWS? seems like a good idea.
Comment 5 by Stephen Leake, Jan 12, 2011
there are several tests failing, mostly because setup now registers databases automatically. Some other failures are not due to changes in this branch.
Comment 6 by Thomas Keller, Jan 12, 2011
I mentioned the Lua hook in the bugfix entry - I thought this one was not important enough to be displayed somewhere more "prominently". I think Richard was so kind and fixed the failing tests. How much I await a working buildbot setup... I'll look over the documentation improvements tonight.
Comment 7 by Thomas Keller, Jan 14, 2011
I decided to reference the hook in the new managed databases section to which is linked from many locations. Since every database-oriented command uses the hook somehow indirectly as soon as it addresses a database in a managed location, I thought it would be a bit overkill. I also reworked the section a bit and grouped the various references to lua hooks in a list. Please review my English therefor from rev 309d7f514b5483b21e470d833677258ac0ed4a7e and drop me a note when you find this ready to merge.
Comment 8 by Thomas Keller, Jan 14, 2011
I added the missing section about register/unregister workspace in rev a2a3357b0ff17f5fbc0fcda117fb0310125b60da and improved a few ref formattings.
Comment 9 by Stephen Leake, Jan 14, 2011
looks good; I think we can propagate this to main now.
Comment 10 by Thomas Keller, Jan 14, 2011
Done. Thanks for the review!
Sign in to reply to this comment.
Reported by Stephen Leake, Jan 9, 2011