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

Modify the edit command and parser for teamTags #49

Conversation

chen-jerry-junior
Copy link

Resolve #47
The previous change made for the edit command did not import the PREFIX_TEAM from CliSyntax in EditCommandParser, so edit command for TeamTags is not working.

@chen-jerry-junior chen-jerry-junior added the priority.Medium Nice to have label Mar 20, 2023
@chen-jerry-junior chen-jerry-junior self-assigned this Mar 20, 2023
@chen-jerry-junior chen-jerry-junior added this to the v1.2 milestone Mar 20, 2023
@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (e808e85) 73.56% compared to head (c24bc56) 73.57%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #49      +/-   ##
============================================
+ Coverage     73.56%   73.57%   +0.01%     
  Complexity      452      452              
============================================
  Files            77       77              
  Lines          1407     1408       +1     
  Branches        148      148              
============================================
+ Hits           1035     1036       +1     
  Misses          325      325              
  Partials         47       47              
Impacted Files Coverage Δ
...n/java/teambuilder/logic/commands/EditCommand.java 96.29% <100.00%> (ø)
...va/teambuilder/logic/parser/EditCommandParser.java 93.10% <100.00%> (+0.24%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link

@WillCWX WillCWX left a comment

Choose a reason for hiding this comment

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

LGTM other than the two parts of commented out code.
You can put the reason in later commits (do remember though, otherwise someone might delete it accidentally in their merge commit)

@@ -262,6 +262,7 @@ && getPhone().equals(e.getPhone())
&& getEmail().equals(e.getEmail())
&& getAddress().equals(e.getAddress())
&& getMajor().equals(e.getMajor())
//&& getTeams().equals(e.getTeams())
Copy link

Choose a reason for hiding this comment

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

Is there a reason why this is commented out for now?

Comment on lines +65 to +67
//if (argMultimap.getAllValues(PREFIX_TEAM).isEmpty()) {
// editPersonDescriptor.setTeams(new HashSet<>());
//}
Copy link

Choose a reason for hiding this comment

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

I think you should put a reason above commented out codes so we know why the code is not deleted and also why the code is not implemented.

@WillCWX WillCWX merged commit f0097d7 into AY2223S2-CS2103T-T17-1:master Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority.Medium Nice to have
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Modify the EditCommand for TeamTags
2 participants