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

Adds 'todo list add' command. Closes #1610 #1613

Closed
wants to merge 3 commits into from

Conversation

ypcode
Copy link
Contributor

@ypcode ypcode commented May 23, 2020

Closes #1610

@coveralls
Copy link

coveralls commented May 23, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling c0968b5 on ypcode:issue/1610-add-todo-task-list into f774287 on pnp:dev.

@waldekmastykarz
Copy link
Member

waldekmastykarz commented May 24, 2020

Hero! Thank you for such a quick turnaround. We'll review it shortly

@garrytrinder garrytrinder changed the title Add command todo list add #1610 Adds 'todo list add' command. Closes #1610 May 24, 2020
Copy link
Member

@garrytrinder garrytrinder left a comment

Nicely done @ypcode 👏🏻

Just a few minor changes that I made whilst reviewing. The final branch can be viewed at https://github.com/garrytrinder/office365-cli/tree/issue/1610-add-todo-task-list


## Examples

Add a list called "My task list"
Copy link
Member

@garrytrinder garrytrinder May 24, 2020

Choose a reason for hiding this comment

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

Changed to My task list to be italic to be consistent with other commands such as spo list add and added task.

@@ -442,6 +442,9 @@ nav:
- report servicesusercounts: 'cmd/tenant/report/report-servicesusercounts.md'
- status:
- status list: 'cmd/tenant/status/status-list.md'
- To Do (todo):
- list:
-list add: 'cmd/todo/list/list-add.md'
Copy link
Member

@garrytrinder garrytrinder May 24, 2020

Choose a reason for hiding this comment

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

Added missing space - list add:

interface CommandArgs {
options: Options;
}

Copy link
Member

@garrytrinder garrytrinder May 24, 2020

Choose a reason for hiding this comment

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

Removed extra space

return 'Adds a new Microsoft To Do task list';
}


Copy link
Member

@garrytrinder garrytrinder May 24, 2020

Choose a reason for hiding this comment

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

Removed extra space

displayName: args.options.name
};


Copy link
Member

@garrytrinder garrytrinder May 24, 2020

Choose a reason for hiding this comment

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

Removed extra space

log(vorpal.find(this.name).helpInformation());
log(
`
Examples:

Add a list called "My task list"
${this.name} --name "My task list"
`);
}
Copy link
Member

@garrytrinder garrytrinder May 24, 2020

Choose a reason for hiding this comment

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

Fixed indentation, added task & replaced quotes in "My task list" with chalk, to be consistent with other commands such as spo list add.

    const chalk = vorpal.chalk;
    log(vorpal.find(this.name).helpInformation());
    log(
      ` Examples:

    Add a task list with the name ${chalk.grey('My task list')}
      ${this.name} --name "My task list"
      `);

Copy link
Contributor Author

@ypcode ypcode May 24, 2020

Choose a reason for hiding this comment

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

Damn... 😁 I took extra care to indent and chalk, I took another random in teams commands to do similar... but I am quite happy with such minor adjustments :) I’ll get to the point, some day, you guys will have nothing to fix :p

Copy link
Member

@garrytrinder garrytrinder May 24, 2020

Choose a reason for hiding this comment

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

I've just realised that we are missing a Remark section to state that the endpoint this command uses is in preview. I'll get this added before we merge.

@garrytrinder garrytrinder self-assigned this May 24, 2020
@garrytrinder garrytrinder linked an issue May 25, 2020 that may be closed by this pull request
@garrytrinder
Copy link
Member

garrytrinder commented May 25, 2020

Merged manually. Thank you! 👏

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

Successfully merging this pull request may close these issues.

New command: Add a Microsoft To Do task list
4 participants