I've put a summary of those with some extra explanation at theĮnd of this comment as a small TODO/FIXME list.
Note there are a few (trivial) fail items in here, Run (generated with the help of the fedora-review tool)
DOSBOX DOS EMULATOR HAS STOPPED WORKING FULL
I've done a full review, below is the list of all checks For benchmarking I use relevant subset of Īs discussed by email I'm taking over this review from François. For 0.76.0 using -O3 generally gives ~1-2% performance gain in average FPS and up to 5% improvement in minimum FPS. If -O3 is not acceptable, I am willing to drop it and go back to -O2 - recent benchmarks for upcoming dosbox-staging 0.76.0 show much smaller performance improvement than what I remember from benchmarking 0.75.0. > Adding to and overriding or filtering parts of these flags is permitted if there’s a good reason to do so the rationale for doing so must be documented in the specfile. If you can present benchmarks that show a significant speedup for this particular code, this could be revisited on a case-by-case basis. > Overriding these flags for performance optimizations (for instance, -O3 instead of -O2) is generally discouraged. Also, added a comment to justify -O3 usage to comply with packaging guidelines: I decided to explicitly mark SDL BuildRequires as >= 2.0.2. (using this would require alternating binary, man page, and probably patching desktop entry) We could use "alternatives" mechanism, but I prefer to do it only if users actually request this. I explained why we kept the binary name (and consequently the man page) in this comment: Once fcami will have a moment to review the spec file, we probably should add "Conflicts: dosbox-staging" to dosbox package as well. The only usecase for doing so is development/regression testing - and for that developers are likely to build their own version of DOSBox (either from SVN or our mirror branch in Git). In this case, I don't think users are interested in having both dosbox and dosbox-staging installed at the same time. Both packages must carry Conflicts in this case. > (…) as long as there are no clear cases for both packages to be installed simultaneously, explicit Conflicts are permitted at the packager’s discretion. I think in this case we're ending up in following rule from the guidelines: I wasn't even aware there is "Conflicts:" tag, that can be used.
0 kommentar(er)
