On Mon, 23 Jul 2012, Stephen Warren wrote: > Sorry for the slow response; I've been away on vacation.
Thank you for taking the time to review the patch. >> +import threading > > It'd be better to simply consistently use thread rather than mixing > thread and threading. OK, I'll correct that. >> -version = "15+" >> +version = "16+" > > Functional patches shouldn't touch the version; it should only be bumped > during or right after release. OK. > Hmm. I've made sure that congruity to date can run with almost any > version of libconcord. That change breaks this feature. Isn't there a > way to introduce support for the new libconcord APIs without making it > non-backwards-compatible? Yes, it is certainly possible to do that. However, I think the code would be cleaner if it didn't have to retain support for the older API's, and my assumption would be that most distros would be upgrading Concordance and Congruity in lockstep. That said, if you feel strongly about it, I can put the old API support back in with my next patch revision. >> -def program_callback_imp(count, current, total, context): >> +def program_callback_imp(stage_id, count, current, total, type, context): >> if not context: >> return >> >> + if stage_id == libconcord.LC_CB_STAGE_NUM_STAGES: >> + return >> + >> try: >> - (f, fcontext) = context >> - percent = (current * 100) / total >> - f(False, percent, fcontext) >> + if isinstance(context, ProgramRemotePanelBase): > > That's not very object-oriented; it'd be better to call a > function/method in the context object, and have that function do > whatever is appropriate, rather than having the code query the type of > the context and then act differently. Good point, I'll rework that in the next version. > Uggh. Why can't libconcord parse the file without being attached to the > remote; the file format should be completely standalone and parse-able > "offline". Don't disagree with you there. > This and the related changes will cause a horrible regression in the > utility of the GUI; the poor user will just see more and more tasks > randomly appearing, and have no idea when the end is in sight. congruity > may as well embed a vte (terminal) widget and just run the concordance > application in it given this change. I've complained about this > libconcord API change repeatedly before... I can see your point about the utility of the GUI. Phil, what are your feelings on changing the API a bit to retain the same the usability of Congruity? > If you want, perhaps I can hand off congruity maintenance to you since > you've obviously got some interest in it; I assume that Sourceforge will > let me set up other committers/admins. Let me know if you want, and what > your Sourceforge login ID is. Sure, I wouldn't mind helping with the maintenance. My Sourceforge ID is swt2c. Thanks, Scott ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
