[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: gEDA-dev: Fix for bug #1527465: zoom extents when window ismaximised



Hi Carlos,

Carlos Nieves Ónega writes:
 > [...]
 > I didn't quite understand why you were saying that....
 > I did one commit for each patch, so it should be easy to follow and go
 > back if something is wrong.

Here is how I think it should have been done:

  - Patch 1: alternate fix for Bug#1527465

     1 - revert the changes you made (previous fix) gschem and libgeda
     - this one is not actually in my tree since I have not yet updated.
     2 - clean up of x_event_configure() - this is quite a changes
     and as such I will certainly not garantee it is bug-free.
     3 - apply new fix (possibly after discussion).

  This way we can track each and every changes. We have the
  possibility to go back on any of them, easily identify when a
  particular appeared and maintain a clean history.

  Right now if you want to go back and remove this commit you are back
  with the previous problematic fix. Not a good solution IMHO.

  - Patch 2: filter as user types and clear button

     1 - add a clear button to the filter of the component selection
     dialog
     2 - modify component selection dialog for filter as user types

  - Patch 3: new close dialog

     1 - add new functions to manipulate pages in windows - pretty big
     changes in key places of the code and covers code that you
     modified recently.
     2 - add new dialog for user confirmation before closing page or
     window.

Order of the patches is not significant here.

Please note how the changes are progressive and how easy it would be
to revert one or identify a faulty one (alsmost systematically) and
fix it.


 > In the last commit, I grouped all the ChangeLog entries into one, so
 > there weren't 3 different entries with the same author and the same
 > date. I only removed the date and author line of the other two
 > commitments. Maybe that's the reason you were not so happy?

The date of an entry has to be the date the changes reached the
CVS. No problem with that. However modifying an entry (the date in
this case) at the next commit that is totally unrelated, is to say the
least messy. Plus you forgot include/x_compselect.h on patch 2.

I am open to discussion on this point but to me the ChangeLog must
reflect the changes commited. It means logically grouping files in
entries even if it implies repetitions from one line to another (not
the case here) and avoiding changes on previous things like the
plague.


 > Another comment: it would speed up the process of component placing if
 > double clicking on a component is like selecting it and click on the
 > apply button.
 > One click: select the component.
 > Double click: select the component and place it.

Wrong thread. Anyway, I will see what I can do.

Regards,


Patrick


_______________________________________________
geda-dev mailing list
geda-dev@moria.seul.org
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev