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

Implement a command that lists ongoing schedules #86

Merged

Conversation

sarjinius
Copy link

No description provided.

Copy link

@tahsinhasem tahsinhasem left a comment

Choose a reason for hiding this comment

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

Overall Looks good to me. But I've suggested some small changes!

@tahsinhasem tahsinhasem self-assigned this Apr 4, 2024
@tahsinhasem tahsinhasem added this to the v1.3 milestone Apr 4, 2024
* master: (50 commits)
  Added semicolon to separate regex
  Extracted out getPersonsFRomCsv
  Fixed EOF error for typicalSchedulesScheduleList.json
  Add more test coverage
  Add more test coverage
  Added test case to improve coverage of ImportCommand
  Added test cases for ParserUtil::parseFileForExport
  Fixed checkstyle for FileFormatTest
  Added tests for FileFormatTest
  Fix ERROR:../src/test/data/JsonScheduleStorageTest/typicalSchedulesScheduleList.json:12: no newline at EOF. error
  Fixed checkstyle errors
  bug fix
  Fix checkstyle errors
  Bug fix where files were not being exported to the correct path
  Applied fixes after merge
  Resolved all suggested changes
  Fix bug where app did not allow exporting contacts without a name
  added requireNonNull to getFileFormat
  Added filename and format checking to export command
  Removed "v/" argument. Changed Parser to parse file for export before allowing find and export command.
  ...
Copy link

codecov bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.29%. Comparing base (327340b) to head (63eaf76).

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #86      +/-   ##
============================================
+ Coverage     80.23%   80.29%   +0.06%     
- Complexity      703      706       +3     
============================================
  Files           105      106       +1     
  Lines          2206     2213       +7     
  Branches        212      212              
============================================
+ Hits           1770     1777       +7     
  Misses          369      369              
  Partials         67       67              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@tahsinhasem tahsinhasem left a comment

Choose a reason for hiding this comment

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

Looks Good!

requireNonNull(model);
LocalDateTime currentDateTime = LocalDateTime.now();
DuringDateTimePredicate predicate = new DuringDateTimePredicate(Optional.of(currentDateTime));
model.updateFilteredScheduleList(predicate);

Choose a reason for hiding this comment

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

Good job making the code more readable!!

@@ -30,7 +30,7 @@
public class FindScheduleCommandParser implements Parser<FindScheduleCommand> {
/**
* Parses the given {@code String} of arguments in the context of the FindScheduleCommand
* and returns a FindCScheduleCommand object for execution.
* and returns a FindScheduleCommand object for execution.

Choose a reason for hiding this comment

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

Good job finding that typo!

@sarjinius sarjinius merged commit 606ad3e into AY2324S2-CS2103T-W08-3:master Apr 4, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants