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

[Patrick Bernaud <b-patrick@xxxxxxxxxx>]

Hi Carlos,

Carlos Nieves Ónega writes:
 > if you think I created a mess in the CVS, I apology for it. It was not
 > my intention to create a mess, but to improve the suite. Next time I'll
 > wait until you commit your patches yourself.

Just to make things clear: the problem is not because YOU commited MY
patches. It is about the way changes have been added to the code base.

They really have to be incremental, as small as possible, one feature
at a time and to concern as few places as possible at once. And I do
not think it has been the case with these patches. Hence my comments.


 > It's easy to revert to the previous state, since the last three commits
 > to gschem and libgeda are because of your 3 patches committed. 

You mean with 'cvs admin -o'? It requires admin priviledge on the
server that I doubt I have been granted. Plus it is dangerous and
would require every people the check out again or we will face serious
troubles.

And if you mean reversing the patches and commiting, it means editing
the ChangeLog and I am not keen on doing that (see previous message).

Leaving them alone looks like our best solution.


 > [...]
 > The changes I made to libgeda are not currently in CVS, since I deleted
 > them when committing your patch.

True for libgeda: message in ChangeLog, commited alone.


 > [...]
 > The function x_event_window_state was deleted when committing your
 > patch, thus removing the definition from prototype.h and unconnecting
 > the signal in x_window.c. Result: the previous fix was deleted.

No it is still in the history of x_event.c and there is no mention
anywhere (ChangeLog and history of x_event.c) that it has been
removed and why.


 > [...]
 > If you go back to the right version, you will have the CVS state before
 > commiting your patches. 

No I will have a version with the previous fix, and a
x_event_configure() function that has nothing to do with the more
recent one (cleaned up and new fix). It is very different.


 > [...]
 > I didn't modify the date. It was always the right one (october, 4th).
 > I meant that, before the commitment, the ChangeLog looked like:

See message <20061004201140.9AA751408042@xxxxxxxxxxxxxx> from
geda-cvs:
>>>>
  --- ChangeLog	4 Oct 2006 19:10:29 -0000	1.521
  +++ ChangeLog	4 Oct 2006 20:11:40 -0000	1.522
  @@ -1,4 +1,16 @@
  -2006-10-03  Patrick Bernaud  <b-patrick@xxxxxxxxxx>
  +2006-10-04  Patrick Bernaud  <b-patrick@xxxxxxxxxx>
  +
<<<<
The previous change was about the fix for #1527465, this one for the
exit_dialog.


 > [...]
 > > Plus you forgot include/x_compselect.h on patch 2.
 > 
 > No, you did. Look at the ChangeLog in CVS:
 > 	* src/x_compselect.c, include/x_compselect.h: Added a button to 
 > 	clear the filter entry of the component selection dialog.
 > 	Modified to auto-update the component selection as user types in
 > 	filter entry.
 > 
 > I didn't applied your patch directly. I was reviewing it and made some minor 
 > changes (like this one and adding the a_zoom_extents after calling the new 
 > page hook).
 > 

Indeed yes. Sorry. Thank for adding it.


Again please understand that it is not specifically against you nor it
is something personal. I am spending a lot of time writing these
messages to avoid being rude and to offend anyone.

I hope I succeeded so far but as you have certainly noticed
English is not my native language. To anyone on the list please feel
free to restate my comments to correct, polite English.

My purpose is to say what can be better done, propose changes for the
future and improve things ... or at least discuss how we can do that.


Regards,


Patrick


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