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

Add CreateBudgetCommand #47

Conversation

yu-ming-chen
Copy link

No description provided.

@yu-ming-chen yu-ming-chen added type.Task Something that needs to be done, but not a story, bug, or an epic e.g. Move testing code into a new priority.High Must do labels Oct 11, 2020
@yu-ming-chen yu-ming-chen added this to the v1.2 milestone Oct 11, 2020
@yu-ming-chen yu-ming-chen self-assigned this Oct 11, 2020
@yu-ming-chen yu-ming-chen linked an issue Oct 11, 2020 that may be closed by this pull request
@yu-ming-chen yu-ming-chen changed the title Branch create budget command Add CreateBudgetCommand Oct 11, 2020
Copy link

@sogggy sogggy left a comment

Choose a reason for hiding this comment

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

Just a few nits before approving

@@ -30,7 +31,7 @@ public JsonAdaptedBudget(@JsonProperty("title") String title,
* Converts a given {@code Budget} into this class for Jackson use.
*/
public JsonAdaptedBudget(Budget source) {
title = source.getTitle();
title = source.getName();
Copy link

Choose a reason for hiding this comment

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

change the variable name to 'name'

@@ -42,7 +43,7 @@ public JsonAdaptedBudget(Budget source) {
* @throws IllegalValueException if there were any data constraints violated.
*/
public Budget toModelType() throws IllegalValueException {
Budget budget = new Budget(title, new ArrayList<Expenditure>());
Budget budget = new Budget(new BudgetName(title), new ArrayList<Expenditure>());
Copy link

Choose a reason for hiding this comment

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

change variable name to name

sogggy
sogggy previously approved these changes Oct 11, 2020
Copy link

@sogggy sogggy left a comment

Choose a reason for hiding this comment

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

LGTM!

@codecov-io
Copy link

Codecov Report

Merging #47 into master will decrease coverage by 0.74%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #47      +/-   ##
============================================
- Coverage     51.27%   50.52%   -0.75%     
  Complexity      339      339              
============================================
  Files            93       95       +2     
  Lines          1488     1510      +22     
  Branches        145      148       +3     
============================================
  Hits            763      763              
- Misses          674      696      +22     
  Partials         51       51              
Impacted Files Coverage Δ Complexity Δ
...dress/logic/commands/main/CreateBudgetCommand.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...in/java/seedu/address/logic/parser/ParserUtil.java 85.36% <0.00%> (-11.86%) 14.00 <0.00> (ø)
...rser/mainpageparser/CreateBudgetCommandParser.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...ss/logic/parser/mainpageparser/MainPageParser.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...c/main/java/seedu/address/model/budget/Budget.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...in/java/seedu/address/model/budget/BudgetName.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
.../java/seedu/address/model/util/SampleDataUtil.java 21.42% <0.00%> (ø) 1.00 <0.00> (ø)
.../java/seedu/address/storage/JsonAdaptedBudget.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 103a455...6dafb27. Read the comment docs.

Copy link

@wenhaogoh wenhaogoh left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@sogggy sogggy left a comment

Choose a reason for hiding this comment

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

LGTM

@yu-ming-chen yu-ming-chen merged commit dbee979 into AY2021S1-CS2103T-T11-4:master Oct 11, 2020
@yu-ming-chen yu-ming-chen deleted the branch-CreateBudgetCommand branch October 11, 2020 13:13
@yu-ming-chen yu-ming-chen restored the branch-CreateBudgetCommand branch October 11, 2020 13:35
@yu-ming-chen yu-ming-chen deleted the branch-CreateBudgetCommand branch October 27, 2020 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority.High Must do type.Task Something that needs to be done, but not a story, bug, or an epic e.g. Move testing code into a new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for 'create' command for the main page parser
4 participants