RichTextComposerSignatures::replaceSignature(),
AttachmentModel::addAttachment(), and Item::appendChildItem() are called
for their side effects, and their returned values are not error indicators,
so there's no need to check them.
Details
Details
- Reviewers
mlaurent - Commits
- R94:ff67fd851586: Remove some warnings about unused return values.
Diff Detail
Diff Detail
- Repository
- R94 PIM: Message Library
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
I don't know the rules for changing interfaces.
AttachmentModel::addAttachment() always returns true (and I think that is not a mistake), and only has 1 caller I know of; maybe it should be eliminated?
Removing the return value of Item::appendChildItem() would require a matching change to ItemPrivate::insertChildItem<>(), but that looks OK to me.
The return value from RichTextComposerSignatures::replaceSignature() is used in some places.
Comment Actions
"AttachmentModel::addAttachment() always returns true (and I think that is not a mistake), and only has 1 caller I know of; maybe it should be eliminated?" if it's always return true, the return value is not useful.
"The return value from RichTextComposerSignatures::replaceSignature() is used in some places." if it's use in some place so it's important to test it too here. Perhaps adding a qCWarning when it's return false etc.
Comment Actions
Changes based on review comments
- AttachmentModel::addAttachment() always returns true, which is not useful, so change it into a void function.
- Restore Q_REQUIRED_RESULT to RichTextComposerSignatures::replaceSignature() and add checks of the returned value where they were missing.
- The return value of Item::appendChildItem() is potentially useful, so don't change it into a void function. However, many current callers have no use for the returned value, so remove the Q_REQUIRED_RESULT attribute.