New Masked Calender for GnuCash

John Ralls jralls at ceridwen.us
Tue Nov 22 13:55:47 EST 2016


> On Nov 22, 2016, at 4:28 AM, Amin Aghabeiki <amin.aghabeiki at gmail.com> wrote:
> 
> Hi Everyone
> 
> I make a lots of  change on  GtkCalendar , the change is in this way :
> 
> 1- I replace GtkCalendar with a  new GObject ( GTK like Calendar) ( all the
> external function is like GtkCalendar )
> 2- separate all the calendar base function ( some function that do
> calculation for week month and etc ) , and create a class like interface
> with separated implantation for Gregorian / Jalalian ( and its also
> possible to be  implement in different calendar logic )
> 3- replace the most part that use GtkCalendar and use now gnucash-calendar
> 
> note : all the implementation/Change is in C not CPP ( so its just a struct
> not a class interface)
> 
> ok now  the question :D
> 
> 1- is it posible to merge my change ( I remove GTK Calendar - and add my
> onwn calendar )?
> 2- if answer of first question is false ,please guide me what should I do ?
> 3- which place use GtkCalendar ( except of Gnc-date-picker - gnc date-edit
> ) to replace my own Gnucash-calendar ) ?
> 
> 
> PS1 : I add a path of my change that has GnuCash-Calendar at this e-mail .
> PS2 : the code is not last
> PS3 : it just compile with CMAKE and make not work ( I did not have time
> yet to find the reason )
> 
> at the end , sorry again for my bad english ;-)

Amin,

It looks like your patch is backwards; your changes seem to be the --- set and the original code the +++ set. It's a pretty big change, so for an easier review process please do it as a Github pull request. As you work on creating it there are some required big-picture changes:
* gnc-jalali.[ch] need to move to libqof/qof. libqof compiles first so it mustn't have dependencies in other directories. 
* Don't use 'qof' as a prefix for any new functions, variables, or constants, use 'gnc'. 'QOF' refers to a co-developed project, the Query Object Framework, that we're gradually replacing as we migrate to a true database application.
* Don't comment out code that you don't want to use, remove it. Git makes it easy to bring back anything that been removed and it's much easier to see removed blocks than commented out ones when reviewing a change.
* Provide Doxygen comments on all new functions. Doxygen comments should include at least a brief description of the function's purpose as well as the parameters and return type.
* Provide unit tests on all new or changed functions, using Google Test, also known as gtest, as the testing framework. 
* Use multiple commits to implement changes, explaining in each commit message what that commit does to move the project forward.  Use at least two commits  for GtkCalendar, the first to copy in the existing code and a second with your changes.
* GtkCalendar should go in gnome-utils, not register. 
* New files *must* have the FSF license blurb at the top including a proper copyright statement: "Copyright 2016 Amin Aghabiki <amin.aghabeiki at gmail.com <mailto:amin.aghabeiki at gmail.com>>"
* The way you decomposed the selection of gregorian/jalali in gnc-date.cpp is ugly. For example, instead of having a separate public function GncDate::format_masked for non-gregorian calendars and splattering if/else clauses all over the code, do the branch once in GncDate::format, or better yet subclass GncDate with GncDateJalali and construct the appropriate one based on the preference. Even better, convert GncDate to a template on GncCalendarType and save the vtable indirection. There should be no need to change the C API at all.
* Following up on that, you've created a lot of functions that are copy-pastes of existing functions with a line or two changed. That's poor practice. Extract the common code to a new static function that you call from each of the specializations if you must, but first think about whether the difference would be better handled at a lower abstraction level. 

Regards,
John Ralls




More information about the gnucash-devel mailing list