Skip to content

Eliayazdi week3#3

Open
EliaYazdi wants to merge 5 commits into
masterfrom
eliayazdi-week3
Open

Eliayazdi week3#3
EliaYazdi wants to merge 5 commits into
masterfrom
eliayazdi-week3

Conversation

@EliaYazdi

Copy link
Copy Markdown
Owner

No description provided.

@marianabocoi marianabocoi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove unnecessary code.
Minor comments.

Comment on lines +78 to +79
const todos = await this.read();
let list = [todos];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you do not need to read the todos if you have no need for them.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@nicevo But if we don't read the list we don't have anything to delete or...?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

below you are saying list =[]; so it does not matter what value it had before.
It is specific for removing all todos.

@@ -0,0 +1 @@
[] No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

there are 2 todo.json why do you think that is ? 😄

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ow... I think I committed the list by mistake

@marianabocoi

Copy link
Copy Markdown

there is also a conflict with the master branch
fix with:
git rebase origin/master 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants