aad: new command 'az ad user update'#9297
Conversation
There was a problem hiding this comment.
Is there a better name for this argument? Why not simply have options_list=['--upn', '--object-id']... or break it out into two arguments with distinct help text? My gut tells me we shouldn't have "or" in our argument names.
There was a problem hiding this comment.
But I can see the sdk function is using that as a parameter in the code below.
client.update(upn_or_object_id=upn_or_object_id, parameters=update_parameters)
There was a problem hiding this comment.
This is the named used by SDK, but we should not have exposed it through az ad user show. Since the name has been out there for 2 years, breaking it right now might be a stretch. For better usability, I can add --id as another option, this will get us the consistency across app, sp, and user. Also upn is a logical id for a user, so --id is a right term.
There was a problem hiding this comment.
We can add the split options and deprecate the one with or in it.
There was a problem hiding this comment.
But I can see the sdk function is using that as a parameter in the code below.
client.update(upn_or_object_id=upn_or_object_id, parameters=update_parameters)
There was a problem hiding this comment.
We can check if the older user1 name is not valid any more. Will something like below will do the check
self.check("length([?displayName=='{user1}'])", 0),
|
@yugangw-msft what's the deal with this? Is it still legitimate? |
|
@tjprescott, the PR is still legit. I have addressed your review feedback, can you please take a look? |
src/azure-cli/HISTORY.rst
Outdated
There was a problem hiding this comment.
Should add the new alias to --id from the deprecated option.
The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.