Fix serialization of boolean attrib values#92
Fix serialization of boolean attrib values#92hhussienn wants to merge 1 commit intoboostorg:developfrom hhussienn:bool-serialize-patch
Conversation
Fix serialization of boolean attribute values from '1' or '0' to 'true' or 'false' per this ticket: https://svn.boost.org/trac/boost/ticket/11092
|
I'm helping out with the PR backlog. Looks like you have a code change, no test, and no new examples will be needed. Is this needed, or more semantics? I'm wary to have things change unless there is a need. That isn't to say that this is bad, but I need to be careful with this kind of PR. This is to let you know and help me prioritize PR's. |
|
@kinghussien Can you rebase your PR on devel? |
|
I'm going to close and re-open to trigger a CI build. Question - while this looks sensible (and reasonably innocuous to me) - do we have a way to test this? Based on a quick look, I couldn't see anything in the GraphML spec that requires boolean attributes to be either boolalpha or numeric. I would hope that most software would support both.... anyone have any experience of this? |
|
@jzmaddock gcc6 is timing out a lot. Should it be disabled? I may fork this, rebase on devel, and make a PR in order to close this out. |
|
I've pushed a fix for the timeout to develop. Any thoughts on my comments above? |
|
I don't have experience. Might the graphviz people have any experience? |
|
@jzmaddock Looking into this further, I think NetworkX not working is significant enough to pull in the change. If this does break code, it will be very easy to fix, but surely adds further integration and support with tools in the area. I'll vote to merge it in. |
|
@jzmaddock Could you trigger the failed test? It was a time out. |
Fix serialization of boolean attribute values from '1' or '0' to 'true' or 'false' per this ticket: https://svn.boost.org/trac/boost/ticket/11092