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.41 Attendance bug fixes and UGDG Language corrected #270

Merged
merged 197 commits into from Nov 12, 2018
Merged

v1.41 Attendance bug fixes and UGDG Language corrected #270

merged 197 commits into from Nov 12, 2018

Conversation

meisbokai
Copy link

@meisbokai meisbokai commented Nov 11, 2018

Resolves the following Issues:
[Attendance] viewAttendace wrong argument
Resolves #268
[Attendance] Command parameters
Resolves #267
[Attendance] viewAttendanceDate command
Resolves #266
[Attendance] Repeated lines for invalid date in replaceAtten command
Resolves #256

UGDG has corrected grammer and spelling

Neow Bo Kai and others added 30 commits October 12, 2018 16:58
…port statements to prevent unwanted dependencies.
# Conflicts:
#	src/seedu/addressbook/data/person/Attendance.java
#	src/seedu/addressbook/data/person/Person.java
#	src/seedu/addressbook/parser/Parser.java
#	test/java/seedu/addressbook/logic/LogicTest.java
Import statements allow the use of non-fully qualified names. The use of a fully qualified name which is covered by an import statement is redundant. Consider using the non-fully qualified name.
Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false.
Reassigning values to incoming parameters is not recommended. Use temporary local variables instead.
@coveralls
Copy link

coveralls commented Nov 11, 2018

Pull Request Test Coverage Report for Build 879

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 86.399%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/seedu/addressbook/parser/Parser.java 0 4 0.0%
Totals Coverage Status
Change from base Build 878: 0.0%
Covered Lines: 2414
Relevant Lines: 2794

💛 - Coveralls

@micklwl
Copy link

micklwl commented Nov 11, 2018

Resolves xxx. in your comment to link the issue to your pr.
Refer to #227

@@ -19,7 +19,7 @@
public static final String COMMAND_WORD = "attendance";
public static final String MESSAGE_USAGE = COMMAND_WORD + ":\n"
+ "Updates the attendance of a student. \n"
+ "Parameters: indexOfPerson d/dd-mm-yyyy att/attendance \n"
+ "Parameters: INDEX d/dd-MM-yyyy att/ISPRESENT \n"
+ "\tExample: " + COMMAND_WORD + " " + "1 d/29-09-2018 att/1 \n"
Copy link

Choose a reason for hiding this comment

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

Intriguing, why is only the month capitalized lawl?

Copy link

Choose a reason for hiding this comment

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

I think it should be DATE instead of the format

Copy link
Author

Choose a reason for hiding this comment

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

Mick, u literally told me to change it from date to the format.

Tek In, mm means minutes. MM is month

Copy link

Choose a reason for hiding this comment

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

Its just odd that the rest uppercase everything, while yours is only the month

Copy link
Author

Choose a reason for hiding this comment

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

i'll change all to caps then

Copy link

Choose a reason for hiding this comment

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

Sorry about that, I was comparing with your other commands. However, now that I look through everyone's commands, for PARAMETERS, they are words and not format related

@0WN463
Copy link

0WN463 commented Nov 11, 2018

Also, merge conflict

@0WN463 0WN463 merged commit 3b8fb61 into CS2113-AY1819S1-F10-1:master Nov 12, 2018
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

4 participants