Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003796Doomseeker[All Projects] Cleanuppublic2020-06-05 07:402020-06-08 05:34
ReporterWubTheCaptain 
Assigned ToWubTheCaptain 
PrioritynoneSeverityminorReproducibilityN/A
StatusresolvedResolutionno change required 
PlatformOSOS Version
Product Version1.3.1 
Target VersionFixed in Version 
Summary0003796: qt-json dependency is duplicated to different files (2nd report)
Description
Quote from WubTheCaptain
qt-json (aka Eeli Reilin's JSON library) has code duplication in the Doomseeker source distribution: Once for Doomseeker, and another time for Wadseeker.


I hit this issue again while submitting patches for issue 0003794. The former issue 0003310 is closed, so I'm reporting this again for more discussion due to changes in circumstances. (E.g.: We've also changed from Mercurial to Git, and can use Git submodules like proposed in 0003795. I've also pointed a trivial reason to change those json.{cpp,h} files since that former report.)
Steps To Reproduce
Quote from WubTheCaptain
$ diff -s src/core/json.cpp src/wadseeker/protocols/json.cpp
Files src/core/json.cpp and src/wadseeker/protocols/json.cpp are identical
$ diff -s src/core/json.h src/wadseeker/protocols/json.h
Files src/core/json.h and src/wadseeker/protocols/json.h are identical
Attached Files

- Relationships
related to 0003310closed qt-json dependency is duplicated to different files, deduplicate it 
related to 0003821confirmed Replacing Eili Reilin's JSON library (qt-json) with JSON support in Qt 

-  Notes
User avatar (0021355)
WubTheCaptain (developer)
2020-06-05 08:04

0003794:0021354:
Quote from WubTheCaptain
$ diff -u src/core/json.cpp src/wadseeker/protocols/json.cpp # master branch
--- src/core/json.cpp   2020-06-05 07:48:38.577780264 +0000
+++ src/wadseeker/protocols/json.cpp    2020-06-05 07:48:38.577780264 +0000
@@ -468,11 +468,11 @@
         index = lastIndex + 1;

         if (numberStr.contains('.')) {
-                return QVariant(numberStr.toDouble(NULL));
+                return QVariant(numberStr.toDouble(nullptr));
         } else if (numberStr.startsWith('-')) {
-                return QVariant(numberStr.toLongLong(NULL));
+                return QVariant(numberStr.toLongLong(nullptr));
         } else {
-                return QVariant(numberStr.toULongLong(NULL));
+                return QVariant(numberStr.toULongLong(nullptr));
         }
 }


Though I agree with preferring nullptr over NULL (with little C++ knowledge), my patch in 0003794 would revert this back to NULL to match upstream.

But more importantly, to make my point here, even we haven't been consistent with these duplicated files in our source tree. This wasn't noticed or brought up in 0003310.
User avatar (0021422)
Blzut3 (administrator)
2020-06-08 04:24

Technically would be done as a result of migrating to Qt 5's JSON support, but I guess this ticket can be resolved when that happens.
User avatar (0021423)
WubTheCaptain (developer)
2020-06-08 05:34

I'll resolve this now. OP assumes it would be resolved so that qt-json becomes available from one place, but 0003821 is the apparent solution here.
And 0003821 fixes this issue too, so technically this is not "won't fix" but "no change required".

Issue Community Support
This issue is already marked as resolved.
If you feel that is not the case, please reopen it and explain why.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2020-06-05 07:40 WubTheCaptain New Issue
2020-06-05 07:41 WubTheCaptain Description Updated View Revisions
2020-06-05 07:41 WubTheCaptain Relationship added related to 0003310
2020-06-05 08:04 WubTheCaptain Note Added: 0021355
2020-06-08 04:23 Blzut3 Relationship added related to 0003821
2020-06-08 04:24 Blzut3 Note Added: 0021422
2020-06-08 04:24 Blzut3 Assigned To => Blzut3
2020-06-08 04:24 Blzut3 Status new => acknowledged
2020-06-08 04:24 Blzut3 Assigned To Blzut3 =>
2020-06-08 05:34 WubTheCaptain Note Added: 0021423
2020-06-08 05:34 WubTheCaptain Status acknowledged => resolved
2020-06-08 05:34 WubTheCaptain Resolution open => no change required
2020-06-08 05:34 WubTheCaptain Assigned To => WubTheCaptain






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker