Review TODO/FIXME comments.

AnnotationDialog/Dialog.cpp

@@ -467,7 +467,7 @@ void AnnotationDialog::Dialog::slotCopyPrevious()
     if (m_current < 1)
         return;
 
-    // FIXME: it would be better to compute the "previous image" in a better way, but let's stick with this for now...
+    // (jzarl 2020-07-26): defining the "previous image" as the one before this is the behaviour of the least surprise:
     DB::ImageInfo &old_info = m_editList[m_current - 1];
 
     m_positionableTagCandidates.clear();
@@ -1109,7 +1109,10 @@ void AnnotationDialog::Dialog::slotDeleteImage()
     if (m_setup == SearchMode)
         return;
 
-    if (m_setup == InputMultiImageConfigMode) //TODO: probably delete here should mean remove from selection
+    // should delete mean "remove from selection" or "delete"?
+    // is the user even aware that the dialog is in multi image mode?
+    // IMO (jzarl) this is too ambiguous to do anything other than bail out:
+    if (m_setup == InputMultiImageConfigMode)
         return;
 
     DB::ImageInfoPtr info = m_origList[m_current];

DB/MemberMap.cpp

@@ -142,7 +142,7 @@ QMap<QString, StringSet> MemberMap::groupMap(const QString &category) const
 */
 QStringList MemberMap::calculateClosure(QMap<QString, StringSet> &resultSoFar, const QString &category, const QString &group) const
 {
-    resultSoFar[group] = StringSet(); // Prevent against cykles.
+    resultSoFar[group] = StringSet(); // Prevent against cycles.
     StringSet members = m_members[category][group];
     StringSet result = members;
     for (StringSet::const_iterator it = members.begin(); it != members.end(); ++it) {

DB/NewImageFinder.cpp

@@ -393,7 +393,7 @@ bool NewImageFinder::findImages()
     QElapsedTimer timer;
 
     timer.start();
-    // TODO: maybe the databas interface should allow to query if it
+    // TODO: maybe the database interface should allow to query if it
     // knows about an image ? Here we've to iterate through all of them and it
     // might be more efficient do do this in the database without fetching the
     // whole info.

ImageManager/AsyncLoader.cpp

@@ -64,10 +64,9 @@ void ImageManager::AsyncLoader::init()
     // as that'd mean that a dual-core box would only have one core decoding images, which would be
     // suboptimal.
     // In case of only one core in the computer, use one core for thumbnail generation
-    // TODO(isilmendil): It seems that many people have their images on NFS-mounts.
-    //                   Should we somehow detect this and allocate less threads there?
-    //                   rlk 20180515: IMO no; if anything, we need more threads to hide
-    //                   the latency of NFS.
+    // jzarl:  It seems that many people have their images on NFS-mounts.
+    //         Should we somehow detect this and allocate less threads there?
+    //         rlk 20180515: IMO no; if anything, we need more threads to hide the latency of NFS.
     int desiredThreads = Settings::SettingsData::instance()->getThumbnailBuilderThreadCount();
     if (desiredThreads == 0) {
         desiredThreads = qMax(1, qMin(16, QThread::idealThreadCount() - 1));

Map/GeoCluster.cpp

@@ -102,7 +102,6 @@ Marble::GeoDataLatLonAltBox Map::GeoCluster::boundingRegion() const
 
 Marble::GeoDataCoordinates Map::GeoCluster::center() const
 {
-    // TODO(jzarl): check how this compares to e.g. the center of all coordinates instead:
     return boundingRegion().center();
 }
 

ThumbnailView/ThumbnailModel.cpp

@@ -167,7 +167,6 @@ void ThumbnailView::ThumbnailModel::setImageList(const DB::FileNameList &items)
     preloadThumbnails();
 }
 
-// TODO(hzeller) figure out if this should return the m_imageList or m_displayList.
 DB::FileNameList ThumbnailView::ThumbnailModel::imageList(Order order) const
 {
     if (order == SortedOrder && m_sortDirection == NewestFirst)
@@ -319,8 +318,10 @@ void ThumbnailView::ThumbnailModel::pixmapLoaded(ImageManager::ImageRequest *req
     // The delegate now fetches the newly loaded image from the cache.
 
     DB::ImageInfoPtr imageInfo = DB::ImageDB::instance()->info(fileName);
-    // TODO(hzeller): figure out, why the size is set here. We do an implicit
+    // (hzeller): figure out, why the size is set here. We do an implicit
     // write here to the database.
+    // (jzarl 2020-07-25): when loading a fullsize pixmap, we get the size "for free";
+    // calculating it separately would cost us more than writing to the database from here.
     if (fullSize.isValid() && imageInfo) {
         imageInfo->setSize(fullSize);
     }

XMLDB/Database.cpp

@@ -447,7 +447,7 @@ DB::ImageInfoList XMLDB::Database::takeImagesFromSelection(const DB::FileNameLis
     if (selection.isEmpty())
         return result;
 
-    // iterate over all images (expensive!!) TODO: improve?
+    // iterate over all images (expensive!!)
     for (DB::ImageInfoListIterator it = m_images.begin(); it != m_images.end(); /**/) {
         const DB::FileName imagefile = (*it)->fileName();
         DB::FileNameList::const_iterator si = selection.begin();