Re: Beyond 2.8 - some design thoughts

[Geert Janssens]

Op vrijdag 29 december 2017 18:23:57 CET schreef John Ralls:
> > On Dec 29, 2017, at 8:20 AM, Geert Janssens 
> > wrote:> 
> > Op vrijdag 29 december 2017 10:11:08 CET schreef Alen Siljak:
> >> I'd like to add that, to me, the difference between stable and unstable
> >> version is obvious enough if I see v2.8.0-alpha1, 2.8.0-alpha2,
> >> 2.8.0-beta1, 2.8.0-rc1, and then 2.8.0. I see no need for separate
> >> version
> >> numbers.
> > 
> > That's a good point. I should check though whether our build system can
> > handle this. If it does or if we can make it so, using explicit
> > alpha/beta/rc strings would be very clear. It would be also require some
> > getting used to as until now we never made an explicit distinction
> > between alpha/beta/rc (though we imply it sometimes in warnings). Should
> > we ? And if so, what would be the criterion ?
> 
> I don’t think that the distros would like that scheme. They want suffixes
> separated by a hyphen to be reserved for their own nefarious purposes
> (mostly designating releases with back ported patches from the project’s
> VCS).
> 
> Better, I think, to use x.9y or perhaps x.9yy for unstable releases.
> 
> Regards,
> John Ralls

Fair point too. Keeps our changes to the build system smaller as well.

I believe 10 unstable releases may be a tad sharp (we had 11 for the 2.5 
series). So let's go for the x.9yy scheme instead.

To summarize:

1. our new version scheme will have two numbers x.y

2. The x number will increase for releases with backwards incompatible changes 
(which we typically call major releases)

3. The y number increases for releases with bugfixes or backwards compatible 
improvements (which we typically call bugfix releases)

4. Whenever the x number increases, the y number is reset to 0

5. Major releases are usually preceded by a number of unstable releases to 
allow wider testing and give translators time to translate new and changed 
strings in the upcoming major release. These unstable releases will be 
numbered x.9zz (so the second number can range from 900 to 999).

I believe that is what most have roughly agreed to (or at least have no strong 
objections against).

John also suggested we can skip 2.8.0 and go straight for 3.0. I'm liking that 
idea more and more. It's indeed consistent with the gnome guidelines and 
allows us to start using the new scheme already now.

Any more closing comments before we officially adopt this new scheme ?

Regards,

Geert
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: Beyond 2.8 - some design thoughts

[John Ralls]

> On Dec 29, 2017, at 8:20 AM, Geert Janssens  
> wrote:
> 
> Op vrijdag 29 december 2017 10:11:08 CET schreef Alen Siljak:
>> I'd like to add that, to me, the difference between stable and unstable
>> version is obvious enough if I see v2.8.0-alpha1, 2.8.0-alpha2,
>> 2.8.0-beta1, 2.8.0-rc1, and then 2.8.0. I see no need for separate version
>> numbers.
> 
> That's a good point. I should check though whether our build system can 
> handle 
> this. If it does or if we can make it so, using explicit alpha/beta/rc 
> strings 
> would be very clear. It would be also require some getting used to as until 
> now we never made an explicit distinction between alpha/beta/rc (though we 
> imply it sometimes in warnings). Should we ? And if so, what would be the 
> criterion ?

I don’t think that the distros would like that scheme. They want suffixes 
separated by a hyphen to be reserved for their own nefarious purposes (mostly 
designating releases with back ported patches from the project’s VCS).

Better, I think, to use x.9y or perhaps x.9yy for unstable releases.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[John Ralls]

> On Dec 29, 2017, at 8:55 AM, Geert Janssens  
> wrote:
> 
> Op vrijdag 29 december 2017 17:22:29 CET schreef John Ralls:
>>> On Dec 29, 2017, at 7:41 AM, Geert Janssens 
>>> wrote:> 
>>> Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
 commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
 Author: John Ralls 
 Date:   Tue Dec 26 13:01:50 2017 -0800
 
   Enforce -Werror on C++ files and fix resulting errors.
>>> 
>>> 
>>> 
 diff --git a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
 b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp index
 9402f38..1bfa08e 100644
 --- a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
 +++ b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
 @@ -355,12 +355,12 @@ static void csv_tximp_preview_currency_fmt_sel_cb
 (GtkComboBox* format_selector, info->preview_update_currency_format();
 }
 
 -void csv_tximp_preview_col_type_changed_cb (GtkComboBox* cbox,
 CsvImpTransAssist* info)
 +static void csv_tximp_preview_col_type_changed_cb
 (GtkComboBox* cbox, CsvImpTransAssist* info) {
 
info->preview_update_col_type (cbox);
 
 }
>>> 
>>> Apparently your new compile options require function declarations or to
>>> declare local functions static. What was wrong with the previous behavior
>>> that you choose to enforce this ?
>> 
>> CMakeLists.txt didn’t set any warning or error flags on C++ files, not even
>> -Werror, so I just copied over the C flags and then adjusted them to deal
>> with g++ wanting -Wmissing-declarations instead of -Wmissing-prototypes.
>> 
>> I just now looked at configure.ac and the flags there are rather different,
>> with neither missing-declarations nor missing-prototypes on either C or
>> C++.
>> 
>> configure.ac:
>> AM_CFLAGS=-Wall -Werror -Wno-unused -Wno-deprecated-declarations
>> -Wmissing-prototypes -Wmissing-prototypes
>> -Werror-implicit-function-declaration
>> 
>> AM_CXXFLAGS=-Wall -Werror -Wno-unused
>> 
>> CMakeLists.txt:
>> CMAKE_C_FLAGS=-Werror -Wdeclaration-after-statement -Wno-pointer-sign -Wall
>> -Wunused -Wmissing-prototypes -Wmissing-declarations  -Wno-unused
>> -Wno-deprecated-declarations -std=gnu11 CMAKE_CXX_FLAGS=-std=gnu++11
>> -Werror -Wall -Wmissing-declarations -Wno-unused
>> 
>> I think that we should remove -Wno-unused from CXXFLAGS in general and only
>> use it where needed on external sources. GnuCash C++ code is all new and it
>> should all compile clean. For that code the more warnings the better: It
>> gets the compiler to help us write better code.
>> 
> Ok, I fully agree.
> 
>> I’m in favor of having missing-prototypes/missing-declarations and declaring
>> compilation-unit-local functions static to make their scope explicit.
>> 
> Ok, that's a good motivation. Just to be clear for me, having a function 
> static is different from having a static variable, right ? As I understand it 
> the latter is bad for thread safety, the former only ensures its local scope.

Yes, and static in a class declaration means something else again.

Careless use of shared state will cause thread safety issues, yes. But static 
variables aren’t the main source of the shared state problem: Promiscuous 
passing of pointers to heap/free store variables is. Consider the possibility 
that two threads get the pointer to a particular split from QofContainer and 
proceed to edit it. There’s no mutex in QofInstance’s begin_edit/commit_edit, 
so the results are almost guaranteed to be interesting.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Aaron Laws]

On Fri, Dec 29, 2017 at 12:06 PM, Aaron Laws  wrote:

> Ok, that's a good motivation. Just to be clear for me, having a function
>> static is different from having a static variable, right ? As I
>> understand it
>> the latter is bad for thread safety, the former only ensures its local
>> scope.
>
>
> There two "static"s to keep in mind: static storage duration, and members
> not bound to instances. Static functions are functions that can't be used
> outside the current code file:
>
> static void func () { printf ("can't be used outside " __FILE__); }
> (or something like that.)
>
> I should have also said that this first type of static can be applied to
variables, too!

static int my_int; // can only be used in this translation unit: __FILE__.

I hope I answered your question and didn't add confusion!
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Aaron Laws]

>
> Ok, that's a good motivation. Just to be clear for me, having a function
> static is different from having a static variable, right ? As I understand
> it
> the latter is bad for thread safety, the former only ensures its local
> scope.


There two "static"s to keep in mind: static storage duration, and members
not bound to instances. Static functions are functions that can't be used
outside the current code file:

static void func () { printf ("can't be used outside " __FILE__); }
(or something like that.)

But very different is:

struct A { int a;  static int s_a; };
int A::s_a = 0;
int main () {
   printf ("%d is accessible.\n", A::s_a);
   //printf ("%d\n", A::a); this is illegal
   return 0;
}

Here, A::s_a can be accessed without ever creating an instance of "A".
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Geert Janssens]

Op vrijdag 29 december 2017 17:22:29 CET schreef John Ralls:
> > On Dec 29, 2017, at 7:41 AM, Geert Janssens 
> > wrote:> 
> > Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
> >> commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
> >> Author: John Ralls 
> >> Date:   Tue Dec 26 13:01:50 2017 -0800
> >> 
> >>Enforce -Werror on C++ files and fix resulting errors.
> > 
> > 
> > 
> >> diff --git a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
> >> b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp index
> >> 9402f38..1bfa08e 100644
> >> --- a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
> >> +++ b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
> >> @@ -355,12 +355,12 @@ static void csv_tximp_preview_currency_fmt_sel_cb
> >> (GtkComboBox* format_selector, info->preview_update_currency_format();
> >> }
> >> 
> >> -void csv_tximp_preview_col_type_changed_cb (GtkComboBox* cbox,
> >> CsvImpTransAssist* info)
> >> +static void csv_tximp_preview_col_type_changed_cb
> >> (GtkComboBox* cbox, CsvImpTransAssist* info) {
> >> 
> >> info->preview_update_col_type (cbox);
> >> 
> >> }
> > 
> > Apparently your new compile options require function declarations or to
> > declare local functions static. What was wrong with the previous behavior
> > that you choose to enforce this ?
> 
> CMakeLists.txt didn’t set any warning or error flags on C++ files, not even
> -Werror, so I just copied over the C flags and then adjusted them to deal
> with g++ wanting -Wmissing-declarations instead of -Wmissing-prototypes.
> 
> I just now looked at configure.ac and the flags there are rather different,
> with neither missing-declarations nor missing-prototypes on either C or
> C++.
> 
> configure.ac:
> AM_CFLAGS=-Wall -Werror -Wno-unused -Wno-deprecated-declarations
> -Wmissing-prototypes -Wmissing-prototypes
> -Werror-implicit-function-declaration
> 
> AM_CXXFLAGS=-Wall -Werror -Wno-unused
> 
> CMakeLists.txt:
> CMAKE_C_FLAGS=-Werror -Wdeclaration-after-statement -Wno-pointer-sign -Wall
> -Wunused -Wmissing-prototypes -Wmissing-declarations  -Wno-unused
> -Wno-deprecated-declarations -std=gnu11 CMAKE_CXX_FLAGS=-std=gnu++11
> -Werror -Wall -Wmissing-declarations -Wno-unused
> 
> I think that we should remove -Wno-unused from CXXFLAGS in general and only
> use it where needed on external sources. GnuCash C++ code is all new and it
> should all compile clean. For that code the more warnings the better: It
> gets the compiler to help us write better code.
> 
Ok, I fully agree.

> I’m in favor of having missing-prototypes/missing-declarations and declaring
> compilation-unit-local functions static to make their scope explicit.
> 
Ok, that's a good motivation. Just to be clear for me, having a function 
static is different from having a static variable, right ? As I understand it 
the latter is bad for thread safety, the former only ensures its local scope.

Geert
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Aaron Laws]

On Fri, Dec 29, 2017 at 11:22 AM, John Ralls  wrote:

>
>
> I think that we should remove -Wno-unused from CXXFLAGS in general and
> only use it where needed on external sources. GnuCash C++ code is all new
> and it should all compile clean. For that code the more warnings the
> better: It gets the compiler to help us write better code.


I agree. I use '-Wall -Wextra -Wpedantic -Wconversion -Werrro' in my
projects (because -Wall is really -Wsome), but adding them to old code can
result in quite a mess.
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[John Ralls]

> On Dec 29, 2017, at 7:41 AM, Geert Janssens  
> wrote:
> 
> Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
>> commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
>> Author: John Ralls 
>> Date:   Tue Dec 26 13:01:50 2017 -0800
>> 
>>Enforce -Werror on C++ files and fix resulting errors.
>> 
> 
> 
> 
>> diff --git a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
>> b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp index
>> 9402f38..1bfa08e 100644
>> --- a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
>> +++ b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
>> @@ -355,12 +355,12 @@ static void csv_tximp_preview_currency_fmt_sel_cb
>> (GtkComboBox* format_selector, info->preview_update_currency_format();
>> }
>> 
>> -void csv_tximp_preview_col_type_changed_cb (GtkComboBox* cbox,
>> CsvImpTransAssist* info)
>> +static void csv_tximp_preview_col_type_changed_cb
>> (GtkComboBox* cbox, CsvImpTransAssist* info) {
>> info->preview_update_col_type (cbox);
>> }
>> 
> Apparently your new compile options require function declarations or to 
> declare local functions static. What was wrong with the previous behavior 
> that 
> you choose to enforce this ?

CMakeLists.txt didn’t set any warning or error flags on C++ files, not even 
-Werror, so I just copied over the C flags and then adjusted them to deal with 
g++ wanting -Wmissing-declarations instead of -Wmissing-prototypes.

I just now looked at configure.ac and the flags there are rather different, 
with neither missing-declarations nor missing-prototypes on either C or C++.

configure.ac:
AM_CFLAGS=-Wall -Werror -Wno-unused -Wno-deprecated-declarations 
-Wmissing-prototypes -Wmissing-prototypes -Werror-implicit-function-declaration 

AM_CXXFLAGS=-Wall -Werror -Wno-unused

CMakeLists.txt:
CMAKE_C_FLAGS=-Werror -Wdeclaration-after-statement -Wno-pointer-sign -Wall 
-Wunused -Wmissing-prototypes -Wmissing-declarations  -Wno-unused 
-Wno-deprecated-declarations -std=gnu11
CMAKE_CXX_FLAGS=-std=gnu++11 -Werror -Wall -Wmissing-declarations -Wno-unused

I think that we should remove -Wno-unused from CXXFLAGS in general and only use 
it where needed on external sources. GnuCash C++ code is all new and it should 
all compile clean. For that code the more warnings the better: It gets the 
compiler to help us write better code.

I’m in favor of having missing-prototypes/missing-declarations and declaring 
compilation-unit-local functions static to make their scope explicit.

Regards,
John Ralls



___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: Beyond 2.8 - some design thoughts

[Geert Janssens]

Op vrijdag 29 december 2017 10:11:08 CET schreef Alen Siljak:
> I'd like to add that, to me, the difference between stable and unstable
> version is obvious enough if I see v2.8.0-alpha1, 2.8.0-alpha2,
> 2.8.0-beta1, 2.8.0-rc1, and then 2.8.0. I see no need for separate version
> numbers.

That's a good point. I should check though whether our build system can handle 
this. If it does or if we can make it so, using explicit alpha/beta/rc strings 
would be very clear. It would be also require some getting used to as until 
now we never made an explicit distinction between alpha/beta/rc (though we 
imply it sometimes in warnings). Should we ? And if so, what would be the 
criterion ?

To widen the scope here, I would eventually like to get to a point where 
master (or at least unstable) is always in an almost releasable state. All 
work that's not in that state should be on feature branches. That would then 
make any release we'd make a potential release candidate. But we're far from 
that IMO, because our test coverage is not sufficient to confidently assume 
big changes don't break other parts of the code.

> However, I don't feel strongly about the version numbers as long as they
> make sense. The above is just something I'd instinctively recognise if I
> did not know absolutely anything else about the project at hand. 
> The same goes for major/minor version. For example, in the projects I
> contribute to (at work or privately), I tend to continuously update any
> dependencies that have minor version updates, assuming they contain only
> minor improvements. Bug fixes (the third number) are almost mandatory
> updates as they often correct issues that we identify ourselves. Major
> version change requires more time and effort in checking what has changed
> and hence doesn't get updated readily. That usually involves a migration
> path and coordinated effort. All this information is fairly obvious from
> just those three numbers. 

Indeed. I have been pondering this in the gnucash project. A 3 number 
versioning scheme that adheres to this would also mean we'd need not one but 
two maintenance branches: one for bugfixes and one for backwards compatible 
new features. (And all bugfixes would be upmerged into the the backwards 
compatible new features branch but not the other way around). Given the small 
size of our team I'm not sure the extra effort would be justified for the net 
gain. If gnucash were a public library I would probably more conservative.

Regards,

Geert
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Geert Janssens]

Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
> commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
> Author: John Ralls 
> Date:   Tue Dec 26 13:01:50 2017 -0800
> 
> Enforce -Werror on C++ files and fix resulting errors.
> 



> diff --git a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
> b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp index
> 9402f38..1bfa08e 100644
> --- a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
> +++ b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
> @@ -355,12 +355,12 @@ static void csv_tximp_preview_currency_fmt_sel_cb
> (GtkComboBox* format_selector, info->preview_update_currency_format();
>  }
> 
> -void csv_tximp_preview_col_type_changed_cb (GtkComboBox* cbox,
> CsvImpTransAssist* info)
> +static void csv_tximp_preview_col_type_changed_cb
> (GtkComboBox* cbox, CsvImpTransAssist* info) {
>  info->preview_update_col_type (cbox);
>  }
> 
Apparently your new compile options require function declarations or to 
declare local functions static. What was wrong with the previous behavior that 
you choose to enforce this ?

Geert
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Pull Requests

[Robert Fewell]

I had some time to go through the pull requests I had created and extract
the fixes that I think should be included before the code freeze and
created PR 251. Others can be included if deemed appropriate and good
enough otherwise I will work on them in the new year.

Regards,
 Bob
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: Beyond 2.8 - some design thoughts

[Alen Siljak]

   I'd like to add that, to me, the difference between stable and unstable
   version is obvious enough if I see v2.8.0-alpha1, 2.8.0-alpha2,
   2.8.0-beta1, 2.8.0-rc1, and then 2.8.0. I see no need for separate
   version numbers.

   However, I don't feel strongly about the version numbers as long as
   they make sense. The above is just something I'd instinctively
   recognise if I did not know absolutely anything else about the project
   at hand.

   The same goes for major/minor version. For example, in the projects I
   contribute to (at work or privately), I tend to continuously update any
   dependencies that have minor version updates, assuming they contain
   only minor improvements. Bug fixes (the third number) are almost
   mandatory updates as they often correct issues that we identify
   ourselves.
   Major version change requires more time and effort in checking what has
   changed and hence doesn't get updated readily. That usually involves a
   migration path and coordinated effort.
   All this information is fairly obvious from just those three numbers.

   Happy New Year!

   Sent: Wednesday, December 27, 2017 at 6:15 PM
   From: "Geert Janssens" 
   To: gnucash-devel@gnucash.org
   Cc: "Alen Siljak" 
   Subject: Re: Beyond 2.8 - some design thoughts
   I do agree up to some point. I consider the scheme I propose to be
   mostly a
   simplified form of the semantic versioning. We will use one level less,
   because we don't distinguish between bugfixes and compatible features.
   I can
   understand why this is suggested for libraries. The reality is we
   haven't been
   doing this for years. Also with every major release (2.6.0, 2.8.0,...)
   we have
   been introducing backwards incompatible changes. According to the
   semantic
   versioning this means we should have updated the first number rather
   than the
   second one. So applying the rules of semantic versioning on gnucash in
   the
   past would have been misleading.
   Geert
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel