> On Jan 28, 2016, at 9:43 AM, gLETTERyYuMEANSj LETTERyOt > <gletteryyumeansjletter...@gmail.com> wrote: > > 2015-05-28 17:15 UTC+02:00, John Ralls <jra...@ceridwen.us>: >> Please don't submit patches to the mailing list. Open a bug in >> Bugzilla.gnome.org or fork our repo on Github, create a branch with your >> patch, and make a pull request. Patches should be written against HEAD in >> the branch you're working on -- which should be master for a feature request, >> especially one that changes the database or the way it's interpreted. > > I have finally found some spare time to submit a patch this way. It is on > > https://github.com/gijut/gnucash > > It compiles without more warning than current version on x86_64, I have tested > it for one hour. > > It does no more prints any debugging line (so that the patch is easier to > read).
BTW your https://github.com/gijut/gnucash/commit/885314788855f74d920e0637ea41b3289f5b55b4 incorrectly copies a GCC bug: https://sourceware.org/bugzilla/show_bug.cgi?id=15366 *and* overrides a #define that belongs to the libc/libc++ headers. (The bug was the result of the GCC folks following an effort by the ISO C committee to tell the ISO C++ committee what to do. The ISO C++ committee ignored them, but the GCC folks didn't. The ISO C committee fixed their overstep in C11. See http://en.cppreference.com/w/Talk:cpp/types/integer. Please remove that commit. Don't change ChangeLog. It's automatically generated from the git log at each release. You removed the code for the auto-read-only threshold. Don't use Timespec in new code, use time64. Don't remove function description comments; do make them Doxygen markup. Don't use magic numbers. Create static const variables (not #defines!) with descriptive names. > > The limits I quote again below still apply: > >>> - The default timezone used for display *should* be specified in the >>> environment variable TZ in the form "[^-+]*+HHMM$", HH and MM being hours >>> and minutes to add to UTC times to get the local time (including the >>> possible daylight saving time correction). >>> >>> - These patches cannot be disabled by configuration (a patch to >>> preferences.glade is lacking). > >> What's the reason for using an environment variable for determining >> timezone? TZ seems not commonly used and it would be trivially easy to add a >> function to get the UTC offset in gnc-date.cpp. > > This is still work to be done. > >> Entry date handling has long been a matter of controversy in GnuCash, and >> switching the nominal time to 11:00 UTC from 00:00 UTC is the simplest >> option to resolve most of it. 11:00 instead of 12:00 because New Zealand's >> summer time offset is +13 hours and the total population of the -12 timezone >> is only a few thousand people. That change is already planned for GnuCash >> 2.8 unless we decide to go in either of the directions Geert mentioned. > > I have now adopted 1100Z as the default time on transaction creation. Time of > loaded transaction are not changed (and they were midnight for gnucash-2.4*). > >> My understanding is that it's unusual for accountants to care much about >> exact transaction times, > > This has been refered to in the thread "Rounding in the price db." about > conversion rates, e.g., in the mail dated Wed, Aug 12, 2015 at 3:27 PM > shown by gnucash.1415818.n4.nabble.com/Rounding-in-the-price-db-td4679860.html > > I can provide references to other recent threads if it helps. > > 2015-05-29 0:28 UTC+02:00, John Ralls <jra...@ceridwen.us>: >>> I think that the coding and debugging style, the TZ environment variable >>> and >>> the lack of patch to preferences.glade are better reasons to disqualify >>> it. >> >> Those are all fixable. Once they're fixed, the patch can be considered for >> master, though as Geert pointed out there's pressure in the other direction >> to remove times entirely from Entered Date. We have to decide which way to >> go. > > The coding style is still problematic; and I did not use the recent > Gnc_Date_Time class, short of time to test it. > > * the one character string ":". In that case, it is replaced by > the current time and date. > > * Any entered date string starting by ":" and longer that ":" will crash > * gnucash. > > * if TZ is not defined, gnucash crashes immediately. The crashes alone make this patch incomplete and unacceptable as-written. That's all I have time for now, and I won't have more for a couple of weeks. In that time, please rewrite your submission taking the above into account and complying with http://wiki.gnucash.org/wiki/CodingStandard. Put it in a feature branch and squash out irrelevant commits. I'll do a deeper review then. But we still haven't decided whether to drop times altogether from date_posted, and this change, even if otherwise perfect, can't merge until we do. Regards, John Ralls _______________________________________________ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
