cli-microsoft365 Align command's options with our naming convention

We should review all commands and ensure that all options match our naming convention:

if the option doesn't have a prefix (eg. id, title), it refers to the last noun in the command name, eg. spo listitem get > last noun = listitem so id and title refer to listitem
if the option needs to point to a property of a different object, it needs to be prefixed to make this clear, eg. listId, listTitle in spo listitem list. Last noun is listitem but we need the user to specify ID of the list, rather than list item, so just prompting for id or title wouldn't make it clear that we're expecting ID or title of the list

Commands that need adjustments:

[x] aad approleassignment add: objectId and displayName should be prefixed with app
[x] aad approleassignment list: objectId and displayName should be prefixed with app
[x] aad approleassignment remove: objectId and displayName should be prefixed with app
[x] aad o365group add: isPrivate should be a flag instead of a boolean option
[x] aad o365group recyclebinitem list: displayName and mailNickname should be prefixed with group
[x] aad o365group teamify: groupId doesn't need the group prefix and could be just id
[x] aad policy list: policyType doesn't need the policy prefix and could be just type
[x] aad sp get: displayName and objectId should be prefixed with app
[x] flow run cancel: flow should be flowName to disambiguate how we expect users to denote the flow, same applies to environment, which should become environmentName
[x] flow run get: flow should be flowName to disambiguate how we expect users to denote the flow, same applies to environment, which should become environmentName
[x] flow run list: flow should be flowName to disambiguate how we expect users to denote the flow, same applies to environment, which should become environmentName
[x] flow run resubmit: flow should be flowName to disambiguate how we expect users to denote the flow, same applies to environment, which should become environmentName
[x] flow disable: environment should be environmentName to make it clear how we expect users to denote the environment
[x] flow enable: environment should be environmentName to make it clear how we expect users to denote the environment
[x] flow export: environment should be environmentName to make it clear how we expect users to denote the environment
[x] flow get: environment should be environmentName to make it clear how we expect users to denote the environment
[x] flow list: environment should be environmentName to make it clear how we expect users to denote the environment
[x] flow remove: environment should be environmentName to make it clear how we expect users to denote the environment
[x] outlook message move: messageId should be id
[x] pa connector export: environment should be environmentName to make it clear how we expect users to denote the environment
[x] pa connector list: environment should be environmentName to make it clear how we expect users to denote the environment
[x] pa solution reference add: path should be projectPath
[x] spfx package generate: packageName should be name
[x] spo app add: scope should be appCatalogScope
[x] spo app deploy: scope should be appCatalogScope
[x] spo app get: scope should be appCatalogScope
[x] spo app install: scope should be appCatalogScope
[x] spo app list: scope should be appCatalogScope
[x] spo app remove: scope should be appCatalogScope
[x] spo app retract: scope should be appCatalogScope
[x] spo app uninstall: scope should be appCatalogScope
[x] spo app upgrade: scope should be appCatalogScope
[x] spo apppage set: pageName should be name
[x] spo cdn policy list: type should be cdnType
[x] spo cdn policy set: type should be cdnType
[x] spo contenttype field set: fieldId should be id
[x] spo customaction add: url should be webUrl
[x] spo customaction clear: url should be webUrl
[x] spo customaction get: url should be webUrl
[x] spo customaction list: url should be webUrl
[x] spo customaction remove: url should be webUrl
[x] spo customaction set: url should be webUrl
[ ] spo feature disable: url should be webUrl, featureId should be id
[ ] spo feature enable: url should be webUrl, featureId should be id
[ ] spo feature list: url should be webUrl
[ ] spo field get: fieldTitle should be title
[ ] spo field remove: fieldTitle should be title
[ ] spo file checkin: fileUrl should be url
[ ] spo file checkout: fileUrl should be url
[ ] spo file sharinginfo get: url should be fileUrl
[ ] spo folder get: folderUrl should be url
[ ] spo folder remove: folderUrl should be url
[ ] spo folder rename: folderUrl should be url
[ ] spo hubsite connect: url should be siteUrl
[ ] spo hubsite disconnect: url should be siteUrl
[ ] spo hubsite register: url should be siteUrl
[ ] spo hubsite rights grant: url should be hubSiteUrl
[ ] spo hubsite rights revoke: url should be hubSiteUrl
[ ] spo knowledgehub set: url should be siteUrl
[ ] spo list contenttype add: contentTypeId should be id
[ ] spo list contenttype remove: contentTypeId should be id
[ ] spo list view field add: fieldId should be id, fieldTitle should be title, fieldPosition should be position
[ ] spo list view field remove: fieldId should be id, fieldTitle should be title
[ ] spo list view field set: fieldId should be id, fieldTitle should be title, fieldPosition should be position
[ ] spo list view get: viewId should be id, viewTitle should be title
[ ] spo list view remove: viewId should be id, viewTitle should be title
[ ] spo list view set: viewId should be id, viewTitle should be title
[ ] spo listitem list: id should be listId, title should be listTitle
[ ] spo listitem record declare: id should be listItemId
[ ] spo listitem record undeclare: id should be listItemId
[ ] spo page column get: name should be pageName
[ ] spo page column list: name should be pageName
[ ] spo page control get: name should be pageName
[ ] spo page control list: name should be pageName
[ ] spo page control set: name should be pageName
[ ] spo page section add: name should be pageName
[ ] spo page section get: name should be pageName
[ ] spo page section list: name should be pageName
[ ] spo sp grant revoke: grantId should be id
[ ] spo sp permissionrequest approve: requestId should be id
[ ] spo sp permissionrequest deny: requestId should be id
[ ] spo site appcatalog add: url should be siteUrl
[ ] spo site appcatalog remove: url should be siteUrl
[ ] spo site apppermission get: permissionId should be id
[ ] spo site apppermission remove: permissionId should be id
[ ] spo site apppermission set: permissionId should be id
[ ] spo site chrome set: url should be siteUrl
[ ] spo site groupify: siteUrl should be url
[ ] spo site rename: siteUrl should be url
[ ] spo sitedesign rights grant: id should be siteDesignId
[ ] spo sitedesign rights list: id should be siteDesignId
[ ] spo sitedesign rights revoke: id should be siteDesignId
[ ] spo sitedesign task get: taskId should be id
[ ] spo sitedesign task remove: taskId should be id
[ ] spo tenant recyclebinitem remove: url should be siteUrl
[ ] spo tenant recyclebinitem restore: url should be siteUrl
[ ] spo web add: webUrl should be url
[ ] spo web get: webUrl should be url
[ ] spo web list: webUrl should be url
[ ] spo web reindex: webUrl should be url
[ ] spo web remove: webUrl should be url
[ ] spo web set: webUrl should be url
[ ] teams app install: appId should be id
[ ] teams app uninstall: appId should be id
[ ] teams channel get: channelId should be id, channelName should be name
[ ] teams channel remove: channelId should be id, channelName should be name
[ ] teams channel set: channelName should be name
[ ] teams message get: messageId should be id
[ ] teams tab get: tabId should be id, tabName should be name
[ ] teams tab remove: tabId should be id
[ ] teams team archive: teamId should be id
[ ] teams team clone: teamId should be id
[ ] teams team remove: teamId should be id
[ ] teams team set: teamId should be id
[ ] teams team unarchive: teamId should be id
[ ] teams user app add: appId should be id
[ ] teams user app remove: appId should be id
[ ] viva connections app create: appName should be name
[ ] yammer group user add: id should be groupId, userId should be id
[ ] yammer group user remove: id should be groupId, userId should be id
[ ] yammer message like set: id should be messageId
[ ] yammer user get: userId should be id

Sep 09 '21 09:09 waldekmastykarz

In some commands we use filePath and userName as options. While we could shorten them to path and name I wonder if in these cases we should treat the options as a single word. What do you think @pnp/cli-for-microsoft-365-maintainers?

Sep 09 '21 11:09 waldekmastykarz

name is too generic, so I think we should stick with userName and I'm happy with sticking with filePath I don't think we would gain any value from changing that.

Sep 10 '21 06:09 garrytrinder

name is too generic, so I think we should stick with userName and I'm happy with sticking with filePath I don't think we would gain any value from changing that.

Agree with @garrytrinder. We should stick with this naming

Sep 18 '21 06:09 plamber

Across different commands, we also have an inconsistency with the short option name for URL:

sometimes we use -u for --url, --webUrl or --siteUrl
some commands use -w for --webUrl, where -u is used for --url
some commands use -s for --siteUrl, where -u is used for --url

How could we rationalize the use of the -u short so that it makes sense across all commands?`

Sep 24 '21 12:09 waldekmastykarz

I would use url only for all commands

Sep 24 '21 12:09 plamber

Another thing that we should consider and which led to some inconsistencies: in some commands where we have multiple URLs, we chose to explicitly prefix each URL option to disambiguate them, eg: spo folder get --webUrl --folderUrl. If we were to follow our naming convention, we'd use spo folder get --webUrl --url, because the last URL refers to a folder which is the last noun in the command. The question is though, which one is clearer and in cases where we have multiple sorts of options (names, IDs, titles, URLs) we should choose clarity over consistency?

Sep 24 '21 12:09 waldekmastykarz

@plamber:

I would use url only for all commands

That wouldn't be always possible though:

spo folder get requires webUrl and folderUrl
spo group get, using just URL would indicate that it's the group URL rather than web URL (webUrl)

Sep 24 '21 13:09 waldekmastykarz

So what would be your suggestion?

Add the noun of the object in front of all url commands?

GroupUrl SiteUrl Etc?

Sep 24 '21 13:09 plamber

So what would be your suggestion? Add the noun of the object in front of all url commands?

@plamber I guess that's the question

do we always prefix, even when it's clear what the option is for, eg. spo web get --webUrl instead of spo web get --url
do we prefix only when an option is ambiguous, eg. spo folder get --webUrl --folderUrl instead of spo folder get --webUrl --url because it's not clear what --url is referring to
do we stick to consistency and naming convention and if the option isn't prefixed, it's implicit that it refers to the last noun

Sep 24 '21 13:09 waldekmastykarz

I would go for the last option

Sep 24 '21 13:09 plamber

@pnp/cli-for-microsoft-365-maintainers what do you think?

Sep 24 '21 14:09 waldekmastykarz

Across different commands, we also have an inconsistency with the short option name for URL:

sometimes we use -u for --url, --webUrl or --siteUrl

some commands use -w for --webUrl, where -u is used for --url

some commands use -s for --siteUrl, where -u is used for --url

How could we rationalize the use of the -u short so that it makes sense across all commands?`

@plamber, what about this?

Sep 24 '21 14:09 waldekmastykarz

I would always go withe the first or first two letters of the words.

E.g. siteUrl - su or s

Sep 24 '21 14:09 plamber

We can only use one letter for a short because -su actually means -s -u.

@pnp/cli-for-microsoft-365-maintainers other opinions?

Sep 25 '21 07:09 waldekmastykarz

I would always go withe the first or first two letters of the words.

E.g. siteUrl - su or s

What if we have --listId and --listTitle? Current we solve it by exposing respectively -i and -t. In such cases, -l would be ambiguous because the user wouldn't know if it's list ID or Title. We have this issue in some list(item) commands where we use -l for --listId and -t for --listTitle which isn't obvious or consistent.

Sep 25 '21 07:09 waldekmastykarz

I reviewed all commands and put all inconsistencies in the list.

To recap, here are the things we need to decide:

Do we want to disambiguate similar options (eg. web URL vs. list URL vs. folder URL). Available options:
1. we always prefix options, even when it's clear what the option is for, eg. spo web get --webUrl instead of spo web get --url
2. we prefix only when an option is ambiguous, eg. spo folder get --webUrl --folderUrl instead of spo folder get --webUrl --url because it's not clear what --url is referring to
3. we stick to consistency and naming convention and if the option isn't prefixed, it's implicit that it refers to the last noun
Do we want to normalize the use of shorts across commands? Eg. in the different commands we use -u, -w and -s for URL. Extra consideration, what if we have multiple URL options in the command (webUrl, listUrl, folderUrl):
1. We use -u only for web/site
2. We don't use shorts at all

For 1. @plamber gave his preference to be iii (follow naming convention). What do you think @pnp/cli-for-microsoft-365-maintainers. What about 2.?

Sep 25 '21 08:09 waldekmastykarz

For 1 I agree with consensus so far to use the iii option. For 2 I would pick i

However I also want to sleep on it a night cause it feels complex, when writing scripts consistency is key, or at least for me.

Sep 25 '21 14:09 appieschot

I reviewed all commands and put all inconsistencies in the list.

To recap, here are the things we need to decide:

Do we want to disambiguate similar options (eg. web URL vs. list URL vs. folder URL). Available options:

we always prefix options, even when it's clear what the option is for, eg. spo web get --webUrl instead of spo web get --url

we prefix only when an option is ambiguous, eg. spo folder get --webUrl --folderUrl instead of spo folder get --webUrl --url because it's not clear what --url is referring to

we stick to consistency and naming convention and if the option isn't prefixed, it's implicit that it refers to the last noun

Do we want to normalize the use of shorts across commands? Eg. in the different commands we use -u, -w and -s for URL. Extra consideration, what if we have multiple URL options in the command (webUrl, listUrl, folderUrl):

We use -u only for web/site

We don't use shorts at all

For 1. @plamber gave his preference to be iii (follow naming convention). What do you think @pnp/cli-for-microsoft-365-maintainers. What about 2.?

For 2 I would go with 1

Sep 26 '21 06:09 plamber

To add my thoughts...

I'm not a big fan of using shorts in my scripts, one of the big things I like about CLIs is that each command is descriptive and easy to read, however when using shorts they can introduce ambiguity as to which option is being used, i.e. I am more likely to have to consult the docs to understand what the short relates to. In the majority of cases, we don't use short options in our examples in the docs and prefer the use of the more descriptive long option.

Where there is a potential conflict I think that we should avoid using a short options, if we take the following examples, m365 teams channel get --teamId 00000000-0000-0000-0000-000000000000 --channelId '19:[email protected]' could be rewritten as m365 teams channel get --i 00000000-0000-0000-0000-000000000000 --c '19:[email protected]' to use the available short options, in contrast to m365 teams channel get --teamName "Team Name" --channelName "Channel Name" which uses the name based long options which do not have available shorts.

As a rule I think we should always be explicit in our naming convention, i.e. webUrl, listUrl, folderUrl, teamName, channelName.

This may introduce some duplication between the command name and the command options, however I would say that clarity always wins.

Sep 27 '21 06:09 garrytrinder

Thanks for sharing your thoughts @garrytrinder. With prefixing options, would you prefix them always, even though it's clear what the option is for, eg. spo web get --webUrl vs. spo web get --url or would you prefix it only where there are multiple URLs, IDs, and we want to disambiguate?

Sep 27 '21 07:09 waldekmastykarz

Thanks for sharing your thoughts @garrytrinder. With prefixing options, would you prefix them always, even though it's clear what the option is for, eg. spo web get --webUrl vs. spo web get --url or would you prefix it only where there are multiple URLs, IDs, and we want to disambiguate?

I would always prefix, it might seem overkill for a simple example like you have given, but it provides clarity for more complex examples where you have multiple options that are similar.

Sep 27 '21 12:09 garrytrinder

Understood @garrytrinder. @rabwill, @arjunumenon, @VelinGeorgiev care to share your opinion?

Sep 27 '21 15:09 waldekmastykarz

Prefix all the way. Agree with @garrytrinder . Thanks for putting this up as a list @waldekmastykarz 👏🏽

Sep 28 '21 02:09 rabwill

Been crazy weeks and that's why had been away from all CLI fun.

Do we want to disambiguate similar options (eg. web URL vs. list URL vs. folder URL). Available options:

I am with what @garrytrinder suggested (Option 1), giving full name for the parameter.

we always prefix options, even when it's clear what the option is for, eg. spo web get --webUrl instead of spo web get --url

Though the con with the approach is that, parameter name would be too big. I would personally be ok with that since our parameters are very clear for usage and the users need not refer the documentation for commands which has multiple id's.

For short params, my vote would be for this,

We use -u only for web/site

Never been a fan of short params myself. So cannot command which would be beneficial.

@waldekmastykarz - You have done heavy fork-lifting by identifying the commands. Awesome ❤

Sep 28 '21 07:09 arjunumenon

With having prefixes and renames, old ones will still be working as aliases maybe? in case someone is using it heavily?

Sep 28 '21 13:09 ValerasNarbutas

@ValerasNarbutas the idea is to introduce these breaking changes in a major version so that we don't need to carry the existing option names forward. Having them available side-by-side would significantly complicate both the codebase and the documentation.

Sep 28 '21 14:09 waldekmastykarz

Since the discussion isn't complete and we consider changing our naming convention, we decided to postpone these changes to the next major version.

Oct 01 '21 07:10 waldekmastykarz

To better understand the impact of prefixing all options with the command noun, let's have a look at some of the examples from our docs:

m365 spo web add --webTitle Subsite --webDescription Subsite --webUrl subsite --webTemplate STS#0 --parentWebUrl https://contoso.sharepoint.com --webLocale 1033

m365 spo site add --siteType ClassicSite --siteUrl https://contoso.sharepoint.com/sites/team --siteTitle Team --siteOwners [email protected] --siteTimeZone 4 --webTemplate STS#0 --removeDeletedSite --wait

m365 spo serviceprincipal permissionrequest approve --permissionRequestId 4dc4c043-25ee-40f2-81d3-b3bf63da7538

m365 aad groupsettingtemplate get --groupSettingTemplateId 62375ab9-6b52-47ed-826b-58e47e0e304b

m365 aad app get --appObjectId d75be2e1-0204-4f95-857d-51a37cf40be8

m365 search externalconnection add --externalConnectionId MyApp --externalConnectionName "My application" --externalConnectionDescription "Description of your application"

Looking at these examples I'm seeing two things:

examples and help would be harder to read because all (or at least most) options start with the same prefix. This makes it harder to scan the help to find the option you're looking for in help
there is a lot of repetition making the commands very verbose. Especially if you're not using command completion in your shell, that's a lot of extra typing

Does prefixing all options add clarity? Absolutely! But for the few cases where we need to disambiguate some options, I wonder if it makes to extend all options in all commands to be prefixed with the command noun. Looking at the examples above, my preference would be to:

keep the current naming convention
align where we identified misalignment
clarify where we see ambiguity

What do you think @pnp/cli-for-microsoft-365-maintainers?

Dec 24 '21 10:12 waldekmastykarz

I'm happy with the proposal 👍🏻

Great work @waldekmastykarz to provide the detail and clarity 👏

Dec 24 '21 11:12 garrytrinder

Thank you @garrytrinder for bringing up other ideas and constructive discussion on the possible directions 👏

Dec 24 '21 12:12 waldekmastykarz

cli-microsoft365 cli-microsoft365 copied to clipboard

Align command's options with our naming convention

cli-microsoft365
cli-microsoft365 copied to clipboard