This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
This is the first-take review: rectcc.h contains a class RECTPP. I *think* I posted my preference here for file names - that is after the class. So: RECTPP.h would be the correct name. Having said that RECTPP is not any of the ok naming conventions for classes. A quick recap: The GNU standards are acceptable, even though I think their application to C++ is less than effective. Alternative, capitalised meaningful words. (my preference). i.e. rect_pp (GNU) Rectangle (best) within the header, you mix interface and implementation. As I said to Igor in the last week or so, please don't do this except for one-liners. (And there while I will accept it, it's not preferred). void offset(int x, int y); is misnamed IMO. void move(int x, int y); would be more intuitive to someone reading the code. Lastly CINSTALL is deprecated. We renamed setup from cinstall to setup when we created the new cvs repository. Ok, onto the main patch. I don't like the overloading of OnActivate. A query method would be more appropriate. i.e. virtual bool toBeShown() const; Why are you removing the __attribute__ ((noreturn)) in LogSingleton? I'm stopping here. There is enough that will need altering, it's confirmed my opinion. This won't go in as is. It also won't go in as a uber-patch. The different things being done need to be split up, so that we can *sensibly* discuss changing them without you having to reissue a ~50K patch every time. Finally: I don't recall a patch for the OnActivate change coming through. Simply because there was high latency is not a good reason to make an *assumption* about how I would have asked you to submit it. And that assumption you have made is a large contributor to the extra work you are now being asked to make. Rob -- GPG key available at: <http://users.bigpond.net.au/robertc/keys.txt>.
Attachment:
signature.asc
Description: This is a digitally signed message part
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |