Slots: signals in BeerXMLElement subclasses?

[mattiasmaahl]

I was running brewtarget through the GDB and found a lot of warnings that the signal 'void changedName(QString);' is redefined in all the subclasses of BeerXMLElement.

Is this appropriate?
As far as I know (which, granted, is not much) the signal is inherited from the base class.
also there's a comment in BeerXMLElement.h that the signals should not be redefined due to silent crashes?

beerxmlemelement.h:
"""
signals:
/*!
* Passes the meta property that has changed about this object.
* NOTE: when subclassing, be \em extra careful not to create a method with
* the same signature. Otherwise, everything will silently break.
*/
void changed(QMetaProperty, QVariant value = QVariant());
void changedFolder(QString);
void changedName(QString);
"""

style.h:
"""
signals:
//! \brief Emitted when \c name() changes.
void changedName(QString);
"""

Is there a reason to leave them in or can I remove them and create a PR?

just asking cause I want to learn. :)

[mikfire]

I do not think those are needed. They are likely a hold over from one of my attempts to decrease the amount of load on the database. I do not see anything emitting that signal, so I suspect you can safely remove them.

The general rule we've tried to follow is that only the Database class emits signals for data having been changed. There are a few exceptions to that, as a few classes have calculated attributes that are not stored in the database (Recipe and Mash, primarily).

[mattiasmaahl]

ok, I'll fix a PR for that then. :)

[mattiasmaahl]

#478 <- PR submitted for this.