Github user lstav commented on the issue:
https://github.com/apache/accumulo/pull/242
Thanks for the feedback @joshelser, I'll start working on it ASAP. Here is
an answer for the general comments you left outside the code:
- **I see some pojo classes have @XmlAttribute on the member variables, and
others don't? Why the inconsistency (when the name isn't being overridden)?**
The @XmlAttribute only shows up on some of the XML tags which have a different
name than the attribute, that's why not all of the variables have it.
- **Might just be me, but I'd prefer to see consistency on the variable
declaration (one per line) and only set an initial value when it doesn't
involve instantiating a new object. I don't remember what Accumulo's "style"
actually says.** I'll look into what the style says, but I agree with you, I
think I started doing it for some reason and then kept doing it out of habit,
but I can change it to your suggestion if the Accumulo "style" agrees.
- **All of those pojos also leave the member variables public. Can we not
encapsulate them and expose them with getters? I am pretty sure that Jackson
supports this.** It does, I changed to use the public variables at the
suggestion of @ctubbsii, but we could discuss the best approach and change it
to that.
- **Some minor whitespace things. Extra newlines at the start of a method
body which are just bloat.
Any cross-browser testing? e.g. FF, Chrome, IE?** I tried using the code
templates for Accumulo and running verify, so that might have escaped me. I
tested on both FF and Chrome, I couldn't get the proxy to work for IE so I
haven't been able to test it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
