> On Nov 22, 2016, at 4:28 AM, Amin Aghabeiki <amin.aghabe...@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.aghabe...@gmail.com 
<mailto:amin.aghabe...@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


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

Reply via email to