-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added data folder and person class #37
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Run mvn compile
so that the formatter can format everything and tell you where there's errors.
* The email is used as the key to the user table. | ||
*/ | ||
public class Person { | ||
private string email, name, company, job, linkedin; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure we need these to be defined on different lines. Can @acmcarther or @jalexanderqed confirm (or deny) this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alex/Jack might ask you to put Javadoc on the public methods, but l don't know for sure since they are just simple setters/getters. Other than that, LGTM.
Update: just remembered one more thing. See comment below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, can you add a getter for email? Just so it matches what we have in Datastore.
@apluscs the key() method returns the email, should I just change the name of the method? |
Nit: PR title is too long. Use the description for details. Example: google/cargo-raze#134 |
Yes that'd be good! |
This pr is made to replace what was previously approved and merged into #33.