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.

Sign up for GitHub

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

Jump to bottom

add Module.java class and ModuleTest.java #59

Merged
merged 4 commits into from
Mar 12, 2024
Merged

add Module.java class and ModuleTest.java #59

merged 4 commits into from
Mar 12, 2024

Conversation

whitesnowx
Copy link
Collaborator

No description provided.

@codecov Codecov
Copy link

codecov bot commented Mar 12, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 75.36%. Comparing base (5777297) to head (779c1b7).
Report is 1 commits behind head on master.

Files Patch % Lines
...rc/main/java/staffconnect/model/person/Module.java 85.71% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master      #59      +/-   ##
============================================
+ Coverage     75.26%   75.36%   +0.10%     
- Complexity      419      424       +5     
============================================
  Files            71       72       +1     
  Lines          1338     1352      +14     
  Branches        126      127       +1     
============================================
+ Hits           1007     1019      +12     
- Misses          301      303       +2     
  Partials         30       30

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

tsulim
tsulim approved these changes Mar 12, 2024
View reviewed changes
Copy link
Collaborator

@tsulim tsulim 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 to me!

@tsulim tsulim merged commit 8a83534 into AY2324S2-CS2103-F08-3:master Mar 12, 2024
5 checks passed
Pluiexo
Pluiexo reviewed Mar 12, 2024
View reviewed changes
src/test/java/staffconnect/model/person/ModuleTest.java
assertFalse(Module.isValidModule("cs2103t")); // suffix not capitalised
assertFalse(Module.isValidModule("cs2103T")); // prefix not capitalised

// valid phone numbers
Copy link

@Pluiexo Pluiexo Mar 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Nits misleading phone numbers comments in this file, please fix in future commits!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will do

Pluiexo
Pluiexo reviewed Mar 12, 2024
View reviewed changes
Copy link

@Pluiexo Pluiexo left a comment

Choose a reason for hiding this comment

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

Nits misleading comments in the file

@tsulim tsulim linked an issue Mar 12, 2024 that may be closed by this pull request
Create Module class #46
Closed
3 tasks
@tsulim tsulim added type.Task Something that needs to be done, but not a story, bug, or an epic. priority.High Must do labels Mar 12, 2024
@tsulim tsulim added this to the v1.2 milestone Mar 12, 2024
@tsulim tsulim removed the priority.High Must do label Mar 12, 2024
@whitesnowx whitesnowx self-assigned this Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pluiexo Pluiexo Pluiexo left review comments

@tsulim tsulim tsulim approved these changes

Assignees

@whitesnowx whitesnowx

Labels
type.Task Something that needs to be done, but not a story, bug, or an epic.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

Create Module class
3 participants
@whitesnowx @Pluiexo @tsulim