<div id="geary-body" dir="auto"><div>Hi all,</div></div><div id="geary-quote" dir="auto">On Thu, Apr 30, 2020 at 18:18, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:<br><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">On Thu, Apr 30, 2020 at 04:05:56PM +0100, Kieran Bingham wrote:
<blockquote> On 30/04/2020 15:15, Umang Jain wrote:
 > This commit introduces no functional changes.
 > This is required so that the combo-box list can be managed
 > conveniently from various private functions in subsequent
 > commit.
 > 
 > Signed-off-by: Umang Jain <<a href="mailto:email@uajain.com">email@uajain.com</a>>
 > ---
 >  src/qcam/main_window.cpp | 9 ++++-----
 >  src/qcam/main_window.h   | 2 ++
 >  2 files changed, 6 insertions(+), 5 deletions(-)
 > 
 > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
 > index d021fa9..344b7ec 100644
 > --- a/src/qcam/main_window.cpp
 > +++ b/src/qcam/main_window.cpp
 > @@ -11,7 +11,6 @@
 >  #include <string>
 >  #include <sys/mman.h>
 >  
 > -#include <QComboBox>
 
 Shouldn't we keep this here ... and...
 
 >  #include <QCoreApplication>
 >  #include <QFileDialog>
 >  #include <QImage>
 > @@ -114,14 +113,14 @@ int MainWindow::createToolbars()
 >   connect(action, &QAction::triggered, this, &MainWindow::quit);
 >  
 >   /* Camera selector. */
 > - QComboBox *cameraCombo = new QComboBox();
 > - connect(cameraCombo, QOverload<int>::of(&QComboBox::activated),
 > + cameraCombo_ = new QComboBox();
 > + connect(cameraCombo_, QOverload<int>::of(&QComboBox::activated),
 >           this, &MainWindow::switchCamera);
 >  
 >   for (const std::shared_ptr<Camera> &cam : cm_->cameras())
 > -         cameraCombo->addItem(QString::fromStdString(cam->name()));
 > +         cameraCombo_->addItem(QString::fromStdString(cam->name()));
 >  
 > - toolbar_->addWidget(cameraCombo);
 > + toolbar_->addWidget(cameraCombo_);
 >  
 >   toolbar_->addSeparator();
 >  
 > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
 > index 5d6251c..28c325f 100644
 > --- a/src/qcam/main_window.h
 > +++ b/src/qcam/main_window.h
 > @@ -9,6 +9,7 @@
 >  
 >  #include <memory>
 >  
 > +#include <QComboBox>
 
 instead just forward declare
 
        class QComboBox;
 
 But as this hasn't been done for other parts of this component, I don't
 think it makes much difference.
 
 Either way...
 
 Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>>
</blockquote>
I was going to mention the same. We already forward-declare QAction that
way. QToolBar should do the same, and so should QComboBox here. Apart
from that,

Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>></div></blockquote><span style="white-space: pre-wrap;"><div><span style="white-space: pre-wrap;"><br></span></div>Well, the QToolBar is already forward-declared via #include <QMainWindow></span><div><span style="white-space: pre-wrap;">header, so it's not should not be necessary to foward-declare it here.</span></div><div><br></div><div><span style="white-space: pre-wrap;">Thanks for the comments. I will submit v2 of the patches in a few minutes.</span></div><div><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">
<blockquote> >  #include <QElapsedTimer>
 >  #include <QIcon>
 >  #include <QMainWindow>
 > @@ -72,6 +73,7 @@ private:
 >   /* UI elements */
 >   QToolBar *toolbar_;
 >   QAction *startStopAction_;
 > + QComboBox *cameraCombo_;
 >   ViewFinder *viewfinder_;
 >  
 >   QIcon iconPlay_;
</blockquote>
<div>-- 
</div>Regards,

Laurent Pinchart
</div></blockquote></div></div><img src="https://u15657259.ct.sendgrid.net/wf/open?upn=GCEip0g28ftA9O9fsCR2M7x08El53O4YVYtHuSI-2FrLwtytoSlmO-2FnSq-2B0q807R-2FE4lsF14i-2FFr8b0GYTbZ-2Fx0ron7uoi3m14JFHobCybhMIyFZcnYk6P2LIJqqDFdwlkOBrCoph8qasWnPkpPcaVqi2ZF6UDskQVxPND01yVVzFyXSfAThtYpPERmOP9CHqzy9deYWC08ZETxPf5q37M3c3Tm6L0Xu222EzCc2eb9C4wxt-2BjHtc5a1e0cSnukfFP" alt="" width="1" height="1" border="0" style="height:1px !important;width:1px !important;border-width:0 !important;margin-top:0 !important;margin-bottom:0 !important;margin-right:0 !important;margin-left:0 !important;padding-top:0 !important;padding-bottom:0 !important;padding-right:0 !important;padding-left:0 !important;"/>