updates the office 365 reports documentation#1393
Conversation
|
Thanks! We'll review it shortly 👏 |
VelinGeorgiev
left a comment
VelinGeorgiev
left a comment
There was a problem hiding this comment.
Thank you Patrick @plamber! It looks quite good. I left only one small comment.
I also have a question. @garrytrinder , @plamber do you think we can benefit if we also add the double quotes around the file name so we change it from --outputFile activityusercounts.txt to --outputFile "activityusercounts.txt"?
I am thinking of a case where intern admins blindly follow the examples, but name their file --outputFile my report 1.txt? You can see where I am getting.
|
|
||
| ```sh | ||
| spo report activitypages --period D7 --output json --outputFile 'report.json' | ||
| spo report activitypages --period D7 --output json --outputFile activitypages.jsons |
There was a problem hiding this comment.
Should that be json instead of jsons?
There was a problem hiding this comment.
Corrected the typo
|
Adding quotes makes sense as a default measure. I came across it when using a string with |
|
Patrick @plamber , would you please be able to make these changes before we merge it? |
|
@VelinGeorgiev performed the changes as requested |
|
@waldekmastykarz @VelinGeorgiev: while performing the suggested changes I noticed that we have the same problem for some other commands. Have a look in "project-externalize.md". It is also not using quotes. I would suggest to create a new issue to handle the cleanup of other items too. |
|
Thank you! We'll review shortly. |
|
Agree. |
|
Created a follow-up issue for this: #1410 |
|
@plamber , could you please use double quotes instead of single quotes. I think this is what we agreed to use with Garry. Sorry for the trouble. |
|
@VelinGeorgiev I missed the double quote part. I already fixed it |
VelinGeorgiev
left a comment
VelinGeorgiev
left a comment
There was a problem hiding this comment.
Looks good Patrick (@plamber)! Thank you!
Your work will be merged from here: https://github.com/VelinGeorgiev/office365-cli/tree/report-docs-update
All I did it to merge your four commits into one.
|
Merged manually. Thank you! 👍 |
4 participants
implements #1391