| Message ID | 20200724230841.27838-1-laurent.pinchart@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your work. On 2020-07-25 02:08:39 +0300, Laurent Pinchart wrote: > Hello, > > This small series contains two patches that I've carried in my tree for > some time, and that I think could be useful. Please see patches for > details. The API is influenced by https://doc.qt.io/qt-5/qflags.html but > goes a step further by disallowing some invalid usage of the operators > that Qt silently allows for internal implementation reasons. This gives me a feeling of a solution looking for a problem :-) In my view what one gains in type-safety one loses in the extra complexity in the usage of this interface. LIBCAMERA_FLAGS_ENABLE_OPERATORS() is a none obvious thing to do for example :-) That being said if you and others find it useful I'm not against it and the code in this series is good and does what it says it should. For the whole series, Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > Laurent Pinchart (2): > libcamera: flags: Add type-safe enum-based flags > test: Add tests for the Flags class > > include/libcamera/flags.h | 195 ++++++++++++++++++++++++++++++++ > include/libcamera/meson.build | 1 + > src/libcamera/flags.cpp | 192 ++++++++++++++++++++++++++++++++ > src/libcamera/meson.build | 1 + > test/flags.cpp | 204 ++++++++++++++++++++++++++++++++++ > test/meson.build | 1 + > 6 files changed, 594 insertions(+) > create mode 100644 include/libcamera/flags.h > create mode 100644 src/libcamera/flags.cpp > create mode 100644 test/flags.cpp > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On Sat, Jul 25, 2020 at 10:25:19AM +0200, Niklas Söderlund wrote: > On 2020-07-25 02:08:39 +0300, Laurent Pinchart wrote: > > Hello, > > > > This small series contains two patches that I've carried in my tree for > > some time, and that I think could be useful. Please see patches for > > details. The API is influenced by https://doc.qt.io/qt-5/qflags.html but > > goes a step further by disallowing some invalid usage of the operators > > that Qt silently allows for internal implementation reasons. > > This gives me a feeling of a solution looking for a problem :-) In my > view what one gains in type-safety one loses in the extra complexity in > the usage of this interface. LIBCAMERA_FLAGS_ENABLE_OPERATORS() is a > none obvious thing to do for example :-) I partly agree with you :-) That's why I'm not pushing for this to get merged, but thought it could be useful. > That being said if you and others find it useful I'm not against it and > the code in this series is good and does what it says it should. For the > whole series, > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > Laurent Pinchart (2): > > libcamera: flags: Add type-safe enum-based flags > > test: Add tests for the Flags class > > > > include/libcamera/flags.h | 195 ++++++++++++++++++++++++++++++++ > > include/libcamera/meson.build | 1 + > > src/libcamera/flags.cpp | 192 ++++++++++++++++++++++++++++++++ > > src/libcamera/meson.build | 1 + > > test/flags.cpp | 204 ++++++++++++++++++++++++++++++++++ > > test/meson.build | 1 + > > 6 files changed, 594 insertions(+) > > create mode 100644 include/libcamera/flags.h > > create mode 100644 src/libcamera/flags.cpp > > create mode 100644 test/flags.cpp
Hello, On Sat, Jul 25, 2020 at 02:49:51PM +0300, Laurent Pinchart wrote: > Hi Niklas, > > On Sat, Jul 25, 2020 at 10:25:19AM +0200, Niklas Söderlund wrote: > > On 2020-07-25 02:08:39 +0300, Laurent Pinchart wrote: > > > Hello, > > > > > > This small series contains two patches that I've carried in my tree for > > > some time, and that I think could be useful. Please see patches for > > > details. The API is influenced by https://doc.qt.io/qt-5/qflags.html but > > > goes a step further by disallowing some invalid usage of the operators > > > that Qt silently allows for internal implementation reasons. > > > > This gives me a feeling of a solution looking for a problem :-) In my > > view what one gains in type-safety one loses in the extra complexity in > > the usage of this interface. LIBCAMERA_FLAGS_ENABLE_OPERATORS() is a > > none obvious thing to do for example :-) > > I partly agree with you :-) That's why I'm not pushing for this to get > merged, but thought it could be useful. > I admit the first time I read the test example I had an immediate feeling of "yes, but why?". > > That being said if you and others find it useful I'm not against it and > > the code in this series is good and does what it says it should. For the > > whole series, > > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > Same for me > > > Laurent Pinchart (2): > > > libcamera: flags: Add type-safe enum-based flags > > > test: Add tests for the Flags class > > > > > > include/libcamera/flags.h | 195 ++++++++++++++++++++++++++++++++ > > > include/libcamera/meson.build | 1 + > > > src/libcamera/flags.cpp | 192 ++++++++++++++++++++++++++++++++ > > > src/libcamera/meson.build | 1 + > > > test/flags.cpp | 204 ++++++++++++++++++++++++++++++++++ > > > test/meson.build | 1 + > > > 6 files changed, 594 insertions(+) > > > create mode 100644 include/libcamera/flags.h > > > create mode 100644 src/libcamera/flags.cpp > > > create mode 100644 test/flags.cpp > > -- > Regards, > > Laurent Pinchart > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel