zorba-coders team mailing list archive

[Chris Hillery <chillery+launchpad@xxxxxxxxx>]

Review: Needs Fixing

1. When adding new virtual methods to a public API class, they must be added at the bottom of the class (after any other virtual methods) to maintain ABI compatibility.

2. Couldn't users just call setBaseURI(""), rather than needing this new method?

3. I don't understand the change to static_context::compute_base_uri(); that seems like it will change behaviour even when nobody calls clearBaseURI(). Can you clarify that logic?

Style issues:

4. dataflow_annotations.h: don't comment out code; just delete the line. Also, is the new #include of rewriter_context.h necessary?

5. I don't love the fact that the Windows compilation fixes are part of this merge proposal, since they're unrelated and that won't show up in the log. However, I admit it's a pain to run a separate proposal for them. Could you just add a note to the commit message saying they're there?
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug905028/+merge/104447
Your team Zorba Coders is subscribed to branch lp:zorba.