https://bugs.kde.org/show_bug.cgi?id=374225
--- Comment #16 from Simon <freisi...@gmail.com> --- > --- Comment #15 from Mario Frank <mario.fr...@uni-potsdam.de> --- > Simon, polishing my patches is completely okay. > Just to be safe. Deleting the region, the identity and the connection from tag > to image is okay? I think deleting the region and identity by default, and asking about the tag is a sane behaviour. > Should I sync the metadata or not? What should I adopt in my patch? > Yes, metadata should be synced (always). However this is not done in many (if not all, I didn't check) methods of tagmodificationhelper. The other methods delete tags via AlbumManager and looking at the right tag sidebar, syncing metadata involves some fileworker interfaces. So this seems quite a big and separate fix that is not directly related to this. I discuss/solve that part in a new issue. Do you agree on Gilles proposition to change "Delete person tag" to "Remove Face Tag"? Another question: The patch introduced a new method with an sql query. In the existing code such queries are sometimes done directly via execSql, at other places via get-/exec- DBAction, so from dbconfig.xml.cmake.in . What decides whether a query gets into dbconfig.xml.cmake.in or whether it doesn't? And only for my education, so ignore at will: In the tagmodificationhelper methods, arguments are sometimes passed by reference (lists), sometimes not (single album) while both underlying types are pointers. Any particular reason for that? -- You are receiving this mail because: You are watching all bug changes.