> On May 2, 2011, 10:50 a.m., Dario Freddi wrote: > > solid/solid/devicemanager.cpp, line 21 > > <http://git.reviewboard.kde.org/r/101270/diff/2/?file=15781#file15781line21> > > > > This include should go below devicenotifier.h and devicemanager_p.h, > > ideally below all includes > > Lamarque Vieira Souza wrote: > Any particular reason for that? Most people, including me, are used to > add non-project includes earlier. This can prevents undesired side effects. > Imagine QFile includes lines like: #ifdef DEFINE #define DEFINE2 int #else > #define DEFINE2 long int #endif, and one of my project's header includes a > line #define DEFINE 1, but my DEFINE has nothing to do with the one in QFile > I can trigger a side effect. It is rare but possible. > > Dario Freddi wrote: > Actually, it's the other way round :) Please read > http://techbase.kde.org/Policies/Library_Code_Policy#Getting_.23includes_right > for an insight on this issue > > Lamarque Vieira Souza wrote: > That doc talks about compiler error, compiler error is easy to detect, > side effects are not.
However, such things as duplicate defines should never happen, whereas compiler errors are not easy to work around if you export the headers (even if that is not the case). Moreover, KDE's coding policies clearly define the headers' order to be as mentioned in the doc - however I'll let Kevin or Alex decide on this matter - Dario ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101270/#review3035 ----------------------------------------------------------- On May 1, 2011, 11:27 p.m., Lamarque Vieira Souza wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101270/ > ----------------------------------------------------------- > > (Updated May 1, 2011, 11:27 p.m.) > > > Review request for Solid, Kevin Ottens and Alex Fiestas. > > > Summary > ------- > > solid-hardware does not report my 3G modem's vendor and model names. This > patch fix that: > > [lamarque@evolucao ~]$ solid-hardware details > /sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1 > udi = '/sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1' > parent = '/org/kde/solid/udev' (string) > vendor = 'Sony Ericsson' (string) > product = 'Sony Ericsson MD300' (string) > description = '' (string) > Block.major = 189 (0xbd) (int) > Block.minor = 175 (0xaf) (int) > Block.device = '/dev/bus/usb/002/048' (string) > > The patch basically tests if > /sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1 exists, if affirmative it > creates a UDevDevice, which makes it possible to report the available device > details instead of just ignoring them completely. > > 'solid-hardware list' still does not list the device. I am still not sure if > I should fix that since it would give the false impression the device is > supported. > > The motivation for this change is Plasma NM. The patch allows me inform > modem's vendor and model names > in Mobile Connection Wizard (when creating Gsm/Cdma connections) and in > PinDialog (when requesting PIN unlock code from user). It is basically > usability fix. > > > Diffs > ----- > > solid/solid/backends/udev/udevdevice.cpp 4f34382 > solid/solid/backends/udev/udevmanager.cpp e08fcde > solid/solid/devicemanager.cpp 59f32a7 > > Diff: http://git.reviewboard.kde.org/r/101270/diff > > > Testing > ------- > > I have been using kdelibs-4.6.2 compiled with the patch since yesterday, not > problems so far. > > > Thanks, > > Lamarque Vieira > >
_______________________________________________ Kde-hardware-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-hardware-devel
