-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update ui with javadocs #179
Update ui with javadocs #179
Conversation
Codecov Report
@@ Coverage Diff @@
## master #179 +/- ##
============================================
- Coverage 36.05% 36.02% -0.04%
Complexity 557 557
============================================
Files 173 173
Lines 3309 3312 +3
Branches 391 391
============================================
Hits 1193 1193
- Misses 2042 2045 +3
Partials 74 74
Continue to review full report at Codecov.
|
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.
Looks good, just some minor nits with Javadocs to comply with coding standard
*/ | ||
private Optional<ListPanel<? extends Item>> setProfileTabView() { | ||
return Optional.of(new ProfileListPanel(profileItems)); | ||
} | ||
|
||
/** | ||
* todo javadocs |
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.
Javadoc could perhaps start with Switches.... depending on
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.
resolved in the latest push.
@@ -1,5 +1,8 @@ | |||
package seedu.address.ui.cards; | |||
|
|||
/** |
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.
Perhaps you could change it to Represents a class...
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.
resolved in the latest push.
*/ | ||
private void setInformationTitle() { | ||
Object jobTitle = mapping.get(TITLE_DISPLAY_NAME); | ||
setInformationTitle(jobTitle.toString()); | ||
} | ||
|
||
/** | ||
* todo javadocs | ||
* Set the information of the {@code profileItem} for display. |
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.
Start with sets..
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.
resolved in the latest push.
…into update-ui-with-javadocs � Conflicts: � src/main/java/seedu/address/ui/display/ApplicationDisplay.java � src/main/java/seedu/address/ui/display/CompanyDisplay.java � src/main/java/seedu/address/ui/display/ProfileDisplay.java
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.
LGTM
Pr Overview