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