Skip to content
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

V1.3 Enhancement #157

Merged
merged 45 commits into from Nov 1, 2018
Merged

V1.3 Enhancement #157

merged 45 commits into from Nov 1, 2018

Conversation

0WN463
Copy link

@0WN463 0WN463 commented Oct 29, 2018

No description provided.

@0WN463 0WN463 self-assigned this Oct 29, 2018
@coveralls
Copy link

coveralls commented Oct 29, 2018

Pull Request Test Coverage Report for Build 618

  • 84 of 111 (75.68%) changed or added relevant lines in 30 files are covered.
  • 4 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.2%) to 82.777%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/seedu/addressbook/commands/assessment/DeleteAssessmentCommand.java 1 2 50.0%
src/seedu/addressbook/commands/commandformat/indexformat/IndexFormatCommand.java 11 12 91.67%
src/seedu/addressbook/commands/assessment/AddGradesCommand.java 1 4 25.0%
src/seedu/addressbook/commands/assessment/DeleteGradesCommand.java 0 3 0.0%
src/seedu/addressbook/ui/MainWindow.java 0 8 0.0%
src/seedu/addressbook/Main.java 0 11 0.0%
Files with Coverage Reduction New Missed Lines %
src/seedu/addressbook/commands/assessment/AddGradesCommand.java 1 14.29%
src/seedu/addressbook/ui/MainWindow.java 1 0.0%
src/seedu/addressbook/commands/assessment/DeleteGradesCommand.java 2 19.05%
Totals Coverage Status
Change from base Build 616: -0.2%
Covered Lines: 2182
Relevant Lines: 2636

💛 - Coveralls

* Extracts the the target person in the last shown list from the given arguments.
* @throws IndexOutOfBoundsException if the target index is out of bounds of the last viewed listing
*/
protected Assessment getTargetAssessment() throws IndexOutOfBoundsException {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can use assessment index out of bounds exception here instead. I have created it separately.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lawl, I just extracted from yours, cause yours also didn't use it in the master

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha yeah, I edited it later

* *User* defines what access level the *Privilege* object have, which is implemented by *BasicUser*, *TutorUser* and *AdminUser*. +
* *BasicUser* is the class with the lowest access level, and the ancestor to other 2 *User* classes. +
* To create an increasing level of access, each *User* of a higher level inherits from the successively lower one. +
* *User* levels have their own list of new commands they can run, which is appended to the list inherited from their parent. +
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that list is necessary here? Or maybe even in the appendix. Just so that it is clear what each of these privilege levels correspond to.

Copy link
Author

@0WN463 0WN463 Nov 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS2101 no like how it was formatted previously (separate lines). I am obliged to satisfy her needs

@0WN463 0WN463 merged commit c424378 into CS2113-AY1819S1-F10-1:master Nov 1, 2018
@0WN463 0WN463 deleted the Enhancement branch November 2, 2018 04:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants