Chris - Good idea regarding not putting the constant in Metadata. I just committed a change as per your suggestion.
Chris & Bertrand, I hear you regarding limiting code changes to the issue at hand. I'll use separate commits for changes nor related to the issue. - Keith Chris Mattmann wrote: > > Hi Keith, > >> I just committed my first code change. I figured that since the issue >> had >> already been discussed, there was no need for further review. On this >> and >> other issues I'm making my best guesses about how to do things the team's >> way. If I screw up, feel free to correct me. ;) > > Not correcting, just a minor objection (not your fault, it's not like it's > documented anywhere). I think that it makes a bit more sense to put a key > for resourceName into an interface class with String constants, than > putting > the key name in the Metadata class. To me, any key name that you need to > reference outside of the Metadata class, should not be part of Metadata > itself. The Metadata class implements interfaces so that it can inherit > such > externally used keys. In the case of resourceName, I'm not sure exactly > what > that has to do with a generic Metadata container. Could we think of a > better > place to put it, e.g., if it has to do with the parsers, wouldn't it make > more sense perhaps to create a TikaParseKeys interface within > org.apache.tika.metadata, and then have the Metadata class implement that > interface? > > >> >> By the way, how do you feel about changing little things in the files >> along >> the way? For example, my IDE, Intellij Idea, enables me to switch to >> Java 5 >> for loop syntax by the click of a single button. Would it be ok to do >> that >> kind of thing, if I'm already committing that file anyway? > > I favor, small incremental patches/commit that fix the problem as defined, > and nothing else really. So if issue TIKA-XXX reports a problem A, and you > commit a fix that fixes A, but that additionally reformats some files (say > you did it correctly and those files weren't using spaces for indentation, > they were using tabs), I would object to such a commit because you're > mixing > 2 things, and the actual changes are harder to track. If your IntelliJ is > telling you things about Tika to refactor loops to make the code more > readable, I would prefer you create an issue for that, and then we can > track > your progress/commits on that specific issue, as opposed to attaching a > rider to the bill ;) > > Cheers, > Chris > > >> >> Thanks, >> Keith > > ______________________________________________ > Chris Mattmann, Ph.D. > [EMAIL PROTECTED] > Cognizant Development Engineer > Early Detection Research Network Project > > _________________________________________________ > Jet Propulsion Laboratory Pasadena, CA > Office: 171-266B Mailstop: 171-246 > _______________________________________________________ > > Disclaimer: The opinions presented within are my own and do not reflect > those of either NASA, JPL, or the California Institute of Technology. > > > > -- View this message in context: http://www.nabble.com/TIKA-72-Commit-tf4641591.html#a13260530 Sent from the Apache Tika - Development mailing list archive at Nabble.com.
