extra fields feature

[flavienlaurent]

• new @BoundBoxField annotation
• add extraFields in @BoundBox annotation
• BoundBoxProcessor changed to parse extra fields
• Test : processor, writer, integration.

[stephanenicolas]

Hi Flavien,

I will put a few notes about the pull request here :

• we should never allow any warnings in the code. In eclipse, the project should be clean, all the time. It's perfectly legitimate to do a minor commit just to remove a warning.
• I made a few changes to ExtraFieldTest, did you know about the method fail in junit 4?
• It has been impossible for me to run the tests using "mvn clean install". That's why Travis had some bad time. I made a few changes to solve this.
• I pushed your branch, and my review on the extra-fields branch of the original repo. I would still like to review your code, but I think it will be included by tomorrow.

Would you like to official join the "team" of BoundBox, and get the rights on the repo ?

S.

[flavienlaurent]

Hi,

I didn't known about fail in junit4, thanks. I would be glad to join the team!

Flavien

[stephanenicolas]

You 're in. ;)

Welcome aboard Flavien.

I will merge your pull request tonight. I hope so..

S. Le 25 sept. 2013 18:18, "Flavien Laurent" notifications@github.com a écrit :

Hi,

I didn't known about fail in junit4, thanks. I would be glad to join the team!

Flavien

— Reply to this email directly or view it on GitHubhttps://github.com/stephanenicolas/boundbox/pull/7#issuecomment-25101820 .

[stephanenicolas]

I go on with the review :

• I added a few links to your javadoc, and enhanced mine.
• I really appreciate that all sentences end with a point, even in comments. Sorry, I am like that :S
• Nice that you used TODO
• We should try to write BoundBox with case.
• I used lombok to get sure params are non-null
• your tests for the processor are wonderful and crystal clear.
• I removed the tests for the writer : they are useless. You add a field info for each extrafield in the processor. For the writer, there is no difference where the extra fields come from. I agree that the exact testing mechanism of the writer may be confusing : I give names of real class in src/test/java to the class info I build there. But it just helps to get a java representation of the class. The java source file is not used, we write a java class using only the meta data classinfo. It may happen that a java file is used though, but only as a superclass.

Is such a peer review fine for you ?

That's a first class contribution, thanks. :) Happy, you joined the team.

Did you subscribe to the mailing list of the project ?

Stéphane

[stephanenicolas]

Your feature is on master, it has been rebased to preserve the tree shape.

Would you like we use topic branches from now ?

I also reviewed the wiki for your feature. Great !