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

Update DG on the implementation of Meeting #249

Merged
merged 11 commits into from
Apr 15, 2024

Conversation

Pluiexo
Copy link

@Pluiexo Pluiexo commented Apr 13, 2024

Added to use cases for add meeting and delete meeting as well.

Copy link

codecov bot commented Apr 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.55%. Comparing base (5aff9ba) to head (1788497).
Report is 61 commits behind head on master.

❗ Current head 1788497 differs from pull request most recent head 9494519. Consider uploading reports for the commit 9494519 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #249   +/-   ##
=========================================
  Coverage     78.55%   78.55%           
  Complexity      846      846           
=========================================
  Files           121      121           
  Lines          2583     2583           
  Branches        262      262           
=========================================
  Hits           2029     2029           
  Misses          498      498           
  Partials         56       56           

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

Copy link
Collaborator

@iynixil iynixil left a comment

Choose a reason for hiding this comment

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

lgtm! some suggestions and nits

docs/DeveloperGuide.md Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
@Pluiexo
Copy link
Author

Pluiexo commented Apr 14, 2024

Been amended

Copy link
Collaborator

@iynixil iynixil left a comment

Choose a reason for hiding this comment

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

i see that the delete meeting UML has been changed to specific example, will add meeting UML and steps also be changed?
otherwise, lgtm.
some subheadings got changed, not sure if intentional?

docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
@Pluiexo
Copy link
Author

Pluiexo commented Apr 14, 2024

i see that the delete meeting UML has been changed to specific example, will add meeting UML and steps also be changed? otherwise, lgtm. some subheadings got changed, not sure if intentional?

The steps are pretty much the same, in fact they are elaborating the general idea of each step as the content of the arguments are not important at that point

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.

Overall looks good, but I have spotted some inconsistency in the changes made.

docs/DeveloperGuide.md Outdated Show resolved Hide resolved
@tsulim tsulim added the type.Documentation Improvements or additions to documentation label Apr 14, 2024
@tsulim tsulim added this to the v1.4 milestone Apr 14, 2024
@iynixil iynixil merged commit ff605c5 into AY2324S2-CS2103-F08-3:master Apr 15, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type.Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants