rmannibucau commented on PR #84: URL: https://github.com/apache/johnzon/pull/84#issuecomment-1105504570
@dblevins - about the formatting it is just a matter of not changing the indentation when no change were done ([MappingParserImpl.java](https://github.com/apache/johnzon/pull/84/files#diff-fb94af49bd5359016321309fc37e72958ebf59edc056b71223597162ccd6298e) for ex), we don't have a strong enforcement but "common" PR rule to not change what we don't modify applies when possible (not saying we are super clean but having a style enforcer is worse for contributor from my experience on other projects so we didn't set it up) - on the new tests you should cose the parser ;) (same, it leaks mem/buffers) > There is a call to generator.close() in the get() method in the Snippet class. If you're thinking of something else, let me know and I can adjust. Sure, try/finally (or autoclose), no assumption it will be called...cause it can fail in between. You should also not create the generator in the constructor (think primitive cases you hardcoded) but when hitting the root object probably. Another related issue is, I think, we should reuse the json generator factory we already have with its config in the mapper (https://github.com/apache/johnzon/blob/c8e233e2f3c54037b8aa5d5d9bf165b550cc9641/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java#L63) instead of using a banalized one (`Json`/`JsonProviderImpl`). Long story short the idea would be something along this: https://gist.github.com/rmannibucau/bc400921f17dd12eb86491462071b0e9 . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@johnzon.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
