search

cfpb / regulations-parser

(DEPRECATED) Parser for U.S. federal regulations and other regulatory information
Other
54 stars 24 forks source link

Xml writer devel #302

Closed grapesmoker closed 8 years ago

grapesmoker commented 9 years ago

This is a PR for the code that implements an XMLWriter class which is responsible for taking the tree structure produced by the parser, analyzing its layers, and writing the result to an XML file. Apparently it can't be merged automatically, so there needs to be some review of how to do this right. I think the actual conflicts are not particularly bad, but this branch does add a lot of new code and perhaps git can't figure out how to reconcile it automatically. This PR is for master, but I don't know if we'd want to create an xml-writer-devel branch on the cfpb repo for this.

There's also some refactoring of the builder included, which might conflict with some of Will's work. I'll let him be the judge of what to do about that.

willbarton commented 9 years ago

I'm of two minds about a branch on the cfpb repo (though I think I originally suggested it). I In think I'd suggest getting #300 and #301 merged, and then tagging the parser, so we always have a point we can come back to, then just continuing work master. What do you think?

willbarton commented 9 years ago

This looks like a great first-pass! My only general comment would be that I'd like to see some tests. I know api writers don't have the most robust tests as it is, but I think tests would help document what we're trying to do and ensure that the mini-assumptions we're making are valid.

willbarton commented 8 years ago

I'm going to close this. The work has moved to the xml-writer-devel branch on this repo, and though we still have a few tests to implement, we can PR that soon.