On 2014-10-22 10:28-0000 phil rosenberg wrote:

> I've just committed the plstream:fill changes

I just fixed the broken build caused by those
changes with commit cabaf82.

N.B. none of those using the master tip for their daily plotting work
and testing likes broken builds of the master tip so it was
important to get that fix committed in a timely manner.

I was also concerned about the implications of a broken build for git
bisect. However, I just checked using "git help bisect", and the
return code 125 can be used in test bisect scripts to automatically
skip commits with broken builds.  So as long as we are careful with
our bisect scripts to use exit 125 when our script discovers the build
is broken, I don't think the broken build for your commits 1029625 and
e5b1166 is going to be an issue for git bisect.

In light of this new implication with regard to linking, please figure
out in consultation with Andrew if C callbacks is really the approach
you want to take.  If the consensus is still yes, then it means
our C++ users will have to change the linking of their apps and
libraries, but so be it.

> What was the issue with the pltr[0-2] functions? Do they need changing too?

As I said before, to be consistent with what you have done for the
plfill C callback we need to officially deprecate plstream::tr[0-2]
and replace it everywhere in our examples with the C version,
pltr[0-2].  And for this case as well our users would be required to
link their apps differently.  Here are the affected examples:

software@raven> grep plstream:: examples/c++/*
examples/c++/x09.cc:        plstream::tr1, (void *) &cgrid1 );
examples/c++/x09.cc:        plstream::tr1, (void *) &cgrid1 );
examples/c++/x09.cc:        plstream::tr2, (void *) &cgrid2 );
examples/c++/x09.cc:        plstream::tr2, (void *) &cgrid2 );
examples/c++/x09.cc:        plstream::tr2, (void *) &cgrid2 );
examples/c++/x09.cc:            clevelneg, nlevelneg, plstream::tr2, (void *) 
&cgrid2 );
examples/c++/x09.cc:            clevelpos, nlevelpos, plstream::tr2, (void *) 
&cgrid2 );
examples/c++/x16.cc:        plfill, true, plstream::tr1, (void *) &cgrid1 );
examples/c++/x16.cc:        plfill, false, plstream::tr2, (void *) &cgrid2 );
examples/c++/x16.cc:    pls->cont( w, nx, ny, 1, nx, 1, ny, clevel, ns, 
plstream::tr2, (void *) &cgrid2 );
examples/c++/x16.cc:        plfill, false, plstream::tr2, (void *) &cgrid2 );
examples/c++/x16.cc:            plfill, false, plstream::tr2, (void *) &cgrid2 
);
examples/c++/x16.cc:        plfill, false, plstream::tr2, (void *) &cgrid2 );
examples/c++/x22.cc:    pls->vect( u, v, nx, ny, 0.0, plstream::tr2, (void *) 
&cgrid2 );
examples/c++/x22.cc:    pls->vect( u, v, nx, ny, -1.0, plstream::tr2, (void *) 
&cgrid2 );
examples/c++/x22.cc:        -1.0, plstream::tr2, (void *) &cgrid2 );
examples/c++/x22.cc:    pls->cont( z, nr, ntheta, 1, nr, 1, ntheta, clevel, 
nlevel, plstream::tr2, (void *) &cgrid2 );
examples/c++/x22.cc:    pls->vect( u, v, nr, ntheta, 25.0, plstream::tr2, (void 
*) &cgrid2 );
examples/c++/x23.cc:        // Draw the grid using plstream::box

However, I will hold back on changing all these plstream::tr[0-2] uses
to pltr[0-2] until you and Andrew get back to me with your final
decision in light of the just-discovered linking issues with this
approach.

I have also had a look at what you have said in README.release, and I
think it is a mistake to dismiss this change in callbacks as a minor
one since all plshade and plshades users are going to have to change
their source code and link their applications and libraries
differently as well.  But I will figure that out later once you and
Andrew get back to me with your final decision.

> Alan - some docbook changes managed to get into my first commit
(they were staged, but not commited, which I thought was enough to
allow me to swap branches but I guess not). I didn't spot them until
after my second commit. I then did a --amend to remove them from the
second commit, but not the first. Sorry about that. Thinking now,
I assumed it isn't possible to amend after a further commit, but
perhaps it is. Still learning I guess.

Well, the result has a (slightly) messy history with your docbook
changes appearing and then disappearing, but I don't see any real harm
from this since this messy history would not interfere with git bisect.

Since you have pushed this (slightly) messy history, I don't think
we want to do anything more since changing history on something
that is published is definitely not recommended.

So all the remaining remarks use this case as an example, but are
only relevant for the next time one of us inadvertently creates
a messy history and wants to clean it up _before_ they have pushed
that messy history.

When dealing with such a messy history, I think you are right; from
"git help commit" the --amend option is only used to amend the tip of
the current branch.  So once you have made a second commit after a bad
commit, then --amend does allow you to clean up the mistake (as you have
done), but does not create a clean history.

One basic method for cleaning up the messy history on your topic
branch would be to use "git diff" to capture your overall changes for
the two commits (which, of course, would not include any of your
off-topic and premature docbook work), then create a separate topic
branch just with those overall changes.  Then separately commit the
two commits again (but without the docbook stuff since git diff for
your overall two commits would not capture that).  Then merge from the
revised topic branch and push.

However, that is the low-level approach, and git has much more
powerful methods of dealing with messy history. For example, have a
look at "git help rebase" and look over the "INTERACTIVE MODE"
section.  I haven't used that powerful capability myself yet, but it
appears that could have been used this time to get your topic branch
history cleaned up (say by squashing your two commits into one) before
merging to master and pushing it.

In sum, git has a powerful answer for everything so long as you
do whatever is needed before the push.  After the push is too late.

Alan
__________________________
Alan W. Irwin

Astronomical research affiliation with Department of Physics and Astronomy,
University of Victoria (astrowww.phys.uvic.ca).

Programming affiliations with the FreeEOS equation-of-state
implementation for stellar interiors (freeeos.sf.net); the Time
Ephemerides project (timeephem.sf.net); PLplot scientific plotting
software package (plplot.sf.net); the libLASi project
(unifont.org/lasi); the Loads of Linux Links project (loll.sf.net);
and the Linux Brochure Project (lbproject.sf.net).
__________________________

Linux-powered Science
__________________________

------------------------------------------------------------------------------
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel

Reply via email to