A CONTRIBUTING file is considered special by Github, being automatically
linked to whenever an attempted pull request is made. Writing the
formatting changes along with how to submit bug reports might seem
fractured, but the special status of the CONTRIBUTING file means that
users are likely to find it. Additionally, links will be added to the
Readme and building_on_windows files.
The appveyor script can not use a single quoted double quotes string,
like '"C:\Spaces are trouble\"', hence the solution of defining an
environment variable.
appveyor.yml has been renamed to .appveyor.yml to keep consistent with
.travis.yml and to be default hidden.
Done in batch script because Powershell requires execution policy to be
changed to run scripts. Therefore batch files are considered to have a
lower barrier for entry.
Clang-msvc has problems with around 400 "-Wexpansion-to-defined" errors
in easylogging++.h. No flags seem to be able to disable this error.
Clang-cl is generally in an awkward halfway place between clang and
msvc.
Msvc has problems with C4127 "conditional expression is constant" in the
"qvarlengtharray.h", hence it being disabled with /wd4127.
Neither msvc* compiler has proper support for disabling errors in
specific files. It's either QMAKE magic or waiting for proper support.
Notice that the class has an identical var that is used in several
places. It is unknown if the original programmer intended to use the
member var in this function, but what is actually being done is a new
var being created, changed and returned. This may not respect the
original intention, but it does preserve the original mechanics.
Parameters with the same name as class members are ripe for introducing
bugs. The fact that the "this->" syntax is optional creates confusion
for maintaining programmers who can only guess at the intention.
Casting to void should shut up GCC about
error: statement has no effect [-Werror=unused-value]
static_cast<void>() is possible for all current types. Supposedly an old
C style cast could also be used, but they should not be used in modern
C++ code.
GCC gave error
../src/tabcontrollers/MoveCenterTabController.cpp:57:67: error:
operation on
‘((advsettings::MoveCenterTabController*)this)->advsettings::MoveCenterTabController::lastMoveButtonClick[0]’
may be undefined [-Werror=sequence-point]
Because assigning a value to itself twice is undefined behavior. It is
highly likely that intended behavior was to initialize both array
members to the same value at once.
Mostly casts and unsigned to signed comparisons.
Testing to ensure that the int isn't below 0. If it is, it can't be used
in the array later anyway. After we're sure it isn't below 0 it's safe
to static_cast<size_t>().
Currently with /W3 on MSVC there are no warnings. Clang and g++ are not
tested.
The "QMAKE_CFLAGS_WARN_ON -= -W3" and "QMAKE_CXXFLAGS_WARN_ON -= -W3"
are necessary because if they are not removed in the config, the
compiler will give D9025: overriding '/W4' with '/W3' error warnings and
not actually use /W4.
apperror was shadowing another variable in the scope outside of the if
statement, leading to a warning.
Adding an underscore changes the var name without changing the meaning.
In the future a different structure should probably be chosen for the
statements.
The warnings in dummy functions are preventing the project from being
compiled with warnings as errors.
There were two options:
* Using a define to create a noop (chosen option).
* Commenting out the variable name, leaving only the type.
The first option was chosen because the second option would leave out
information from the IDE and compiler, while the first option wouldn't.
The intent is also clearer with the first option, otherwise it would
likely be necessary to leave comments with every single parameter that
was commented out.
Currently the variable expansion sometimes doesn't work when done over
two variables. Inlining the ~dp0 call along with \..\..\ seems to work.
Testing leads me to believe that the Windows command prompt caches
things in a weird way.
The old build scripts used hard coded directory paths, didn't check if
the paths actually existed and didn't differentiate between compiling,
deploying libraries and packaging for release.
The new build scripts are split into three different files. They are all
longer than the old build scripts, but most of the extra lines are
error checking, which the old one lacked.
It was considered writing the build scripts in another language like
Python or Powershell, but the relatively small size of the project and
unlikelihood of the build systems needing to grow massively means that a
very simple imperative language like the Microsoft scripting language is
fine.
Vasily
Robin Appelman
ykeara