On Sun, Nov 29, 2009 at 9:15 AM, Chris Morley <[email protected]> wrote:
> Could you say what is wrong? I already feel guilty enough to correct
> anything faulty in what I have committed, but need to know what the
> error are.
No need to feel guilty -- if it's your code this time, I'm sure it's
not just you. This has been happening frequently and everytime I
update it breaks. I've attached a diff of the changes needed to get
trunk to compile.
> The trunk code is compiled on at least two compilers, on several
> operating systems, built using two or more build systems, sometimes in
> a variety of configurations. Most developers do not have all these
> available to test before they commit. So it is inevitable that minor
> differences will prevent error-free building from time to time in the
> trunk code.
Of course. But if the errors in the diff I've attached work with ANY
compiler, I'd be amazed. this isn't compiler compatibility, this is
just bad code that wasn't compiled before committing.
> Recent examples are: the use of uint, which will not compile in Visual
> C++; std::tr1::shared_ptr which may be declared in <memory>
> <tr1/memory> or a Boost library depending on your system, but it is
> difficult to test all of them on one computer.
True, but calling functions that aren't defined, including extra
qualifiers on functions in headers, etc -- these aren't merely syntax
differences.
> If a "test-before-you-commit" policy requires the developer to
> download a new copy of the SVN repository after each commit and build
> it from scratch, I am not in favour. The time this would take is not
> justified with the uncritical nature of OB. It would not solve many of
> the problems.
"test-before-you-commit" is more of a mind set than letter of the law,
especially with svn (with git it's easier to do because of stashing,
but that another request for another day :-) ). Carefully reviewing
diffs before you commit to make sure one isn't committing code they
aren't ready to commit is an easy way to make sure this works. I don't
buy the idea that one can't commit working code because it's too much
of a hassel. Many developers keep two local repositories of SCM'd code
-- one vanilla, one experimental. This way we can build diffs of the
experimental branch of a new feature and test them in the vanilla
branch before committing from the vanilla branch to ensure that the
code works when we're through.
Again, this would much easier with git's cheap branching... ;-)
> If you meant have a set of tests which should validate before any
> commit, I agree this would be a good idea, but this also would not
> ensure compatibility.
I think that'd be a bit extreme -- committing over svn to sourceforge
is already slow enough without needing to wait for a pre-commit-hook
to finish compiling. Reviewing diffs before pushing should be enough.
> If you want to use development code I think you should expect to
> correct errors. If you don't have write access, a brief note to this
> list, with a patch or a copy of the error messages would be helpful.
I do have write access, but I'm not familiar with the parts of the
code that are failing. All that is needed to see the errors I'm
getting is a clean checkout and compile. I think that if one wants to
commit code to a community repository, taking measures to ensure it
compiles is basic courtesy -- we shouldn't expect the other users to
have to clean up our mistakes. Of course accidents happen, but
expecting devs who aren't familiar with that part of the code to be
able to fix it isn't the right attitude to have.
I've attached a diff that gets trunk to compile. I have no idea what
this diff breaks, because I've just commented out the bits that gcc
complained about it.
Dave
Index: src/formats/mdlformat.cpp
===================================================================
--- src/formats/mdlformat.cpp (revision 3477)
+++ src/formats/mdlformat.cpp (working copy)
@@ -629,8 +629,8 @@
if(mol.GetDimension()==3)
dimension = "3D";
- if(pConv->IsOption("A"))
- AliasData::RevertToAliasForm(mol);
+ // if(pConv->IsOption("A"))
+ // AliasData::RevertToAliasForm(mol);
// line 1: molecule name
ofs << mol.GetTitle() << endl;
@@ -825,7 +825,8 @@
for (k = vdata.begin();k != vdata.end();k++)
{
if ((*k)->GetDataType() == OBGenericDataType::PairData
- && (*k)->GetOrigin()!=local) //internal OBPairData is not written
+ // && (*k)->GetOrigin()!=local //internal OBPairData is not written
+ )
{
HasProperties = true;
//Since partial charges are not output
Index: src/formats/svgformat.cpp
===================================================================
--- src/formats/svgformat.cpp (revision 3477)
+++ src/formats/svgformat.cpp (working copy)
@@ -15,8 +15,8 @@
#include <openbabel/obmolecformat.h>
#include <openbabel/op.h>
#include <openbabel/text.h>
-#include <openbabel/depict/svgpainter.h>
-#include <openbabel/depict/depict.h>
+//#include <openbabel/depict/svgpainter.h>
+//#include <openbabel/depict/depict.h>
#include <openbabel/alias.h>
using namespace std;
@@ -291,22 +291,22 @@
<< "x=\"" << innerX + cellsize * 0.5 << "\" y=\"" << innerY + cellsize - 2.0/nr << "\" >"
<< pmol->GetTitle() << "</text>\n";
- SVGPainter painter(*pConv->GetOutStream(), true, cellsize,cellsize,innerX,innerY);
- OBDepict depictor(&painter);
+ // SVGPainter painter(*pConv->GetOutStream(), true, cellsize,cellsize,innerX,innerY);
+ // OBDepict depictor(&painter);
- if(pConv->IsOption("C"))
- depictor.SetDrawingTerminalCarbon(true);
- if(pConv->IsOption("A"))
- {
- AliasData::RevertToAliasForm(*pmol);
- depictor.SetAliasMode();
- }
- painter.SetFontFamily("Helvetica");
- painter.SetPenColor(OBColor(bondcolor));
- depictor.SetBondColor(bondcolor);
- painter.SetPenWidth(2);
+ // if(pConv->IsOption("C"))
+ // depictor.SetDrawingTerminalCarbon(true);
+ // if(pConv->IsOption("A"))
+ // {
+ // AliasData::RevertToAliasForm(*pmol);
+ // depictor.SetAliasMode();
+ // }
+ // painter.SetFontFamily("Helvetica");
+ // painter.SetPenColor(OBColor(bondcolor));
+ // depictor.SetBondColor(bondcolor);
+ // painter.SetPenWidth(2);
- depictor.DrawMolecule(pmol);
+ // depictor.DrawMolecule(pmol);
//Embed CML of molecule if requested
if(pConv->IsOption("e"))
@@ -318,35 +318,35 @@
//Final </svg> written at the end of this block (painter destructor)
//This leads to some code duplication.
double factor = 1.0;
- SVGPainter painter(*pConv->GetOutStream());
- OBDepict depictor(&painter);
+ // SVGPainter painter(*pConv->GetOutStream());
+ // OBDepict depictor(&painter);
//Scale image by specifying the average bond length in pixels.
if(pConv->IsOption("p"))
{
- double oldblen = depictor.GetBondLength();
+ // double oldblen = depictor.GetBondLength();
double newblen = atof(pConv->IsOption("p"));
- depictor.SetBondLength(newblen);
- factor = newblen / oldblen;
+ // depictor.SetBondLength(newblen);
+ // factor = newblen / oldblen;
//Scale bondspacing and font size by same factor
- depictor.SetBondSpacing(depictor.GetBondSpacing() * factor);
- depictor.SetFontSize((int)(depictor.GetFontSize() * factor));
+ // depictor.SetBondSpacing(depictor.GetBondSpacing() * factor);
+ // depictor.SetFontSize((int)(depictor.GetFontSize() * factor));
}
- if(pConv->IsOption("C"))
- depictor.SetDrawingTerminalCarbon(true);
+ // if(pConv->IsOption("C"))
+ // depictor.SetDrawingTerminalCarbon(true);
if(pConv->IsOption("A"))
{
- AliasData::RevertToAliasForm(*pmol);
- depictor.SetAliasMode();
+ // AliasData::RevertToAliasForm(*pmol);
+ // depictor.SetAliasMode();
}
- painter.SetFontFamily("Helvetica");
- painter.SetPenColor(OBColor(bondcolor));
- depictor.SetBondColor(bondcolor);
- painter.SetFillColor(OBColor(background));
- painter.SetPenWidth(1);
+ // painter.SetFontFamily("Helvetica");
+ // painter.SetPenColor(OBColor(bondcolor));
+ // depictor.SetBondColor(bondcolor);
+ // painter.SetFillColor(OBColor(background));
+ // painter.SetPenWidth(1);
- depictor.DrawMolecule(pmol);
+ // depictor.DrawMolecule(pmol);
//depictor.AddAtomLabels(OBDepict::AtomSymmetryClass);
@@ -480,4 +480,4 @@
Center the molecules
*/
-}//namespace
\ No newline at end of file
+}//namespace
Index: src/formats/xml/cmlformat.cpp
===================================================================
--- src/formats/xml/cmlformat.cpp (revision 3477)
+++ src/formats/xml/cmlformat.cpp (working copy)
@@ -1892,7 +1892,7 @@
for (k = vdata.begin();k != vdata.end();k++)
{
if ((*k)->GetDataType() == OBGenericDataType::PairData
- && (*k)->GetOrigin() != local //internal OBPairData is not written
+ // && (*k)->GetOrigin() != local //internal OBPairData is not written
&& (*k)->GetAttribute()!= "InChI" //InChI is output in <identifier>
&& (*k)->GetAttribute()!= "PartialCharges")//annotation not needed since partial charges are not output in this format
{
Index: src/formats/fastsearchformat.cpp
===================================================================
--- src/formats/fastsearchformat.cpp (revision 3477)
+++ src/formats/fastsearchformat.cpp (working copy)
@@ -89,7 +89,7 @@
private:
bool ObtainTarget(OBConversion* pConv, vector<OBMol>& patternMols, const string& indexname);
- void FastSearchFormat::AddPattern(vector<OBMol>& patternMols, OBMol patternMol, int idx);
+ void AddPattern(vector<OBMol>& patternMols, OBMol patternMol, int idx);
private:
///big data structure which will remain in memory after it is loaded
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
OpenBabel-Devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openbabel-devel
