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

Align short option in commandgroup contenttype field #6170

Closed
4 tasks
Jwaegebaert opened this issue Jul 22, 2024 · 5 comments
Closed
4 tasks

Align short option in commandgroup contenttype field #6170

Jwaegebaert opened this issue Jul 22, 2024 · 5 comments

Comments

@Jwaegebaert
Copy link
Contributor

Jwaegebaert commented Jul 22, 2024

Noticed that spo contenttype field set has a vague short option for contentTypeId. Usually, contentTypeId would be shortened to i, but here it's c. This is probably because the option id is also available, even though it doesn't have a short option.

I suggest we either remove the short c or change it to i. If we change it to i, we need to ensure it doesn't confuse users with the id option.

ToDo:

Changes to spo contenttype field remove:

  • remove the short option for --contentTypeId
  • change --fieldLinkId <fieldLinkId> to -i, --id <id>

Changes to spo contenttype field set:

  • remove the short option for --contentTypeId
  • add the short -i to --id
@Jwaegebaert Jwaegebaert added enhancement breaking change needs peer review Needs second pair of eyes to review the spec or PR labels Jul 22, 2024
@Jwaegebaert Jwaegebaert added this to the v9 milestone Jul 22, 2024
@milanholemans
Copy link
Contributor

At the set command it's called id while at the remove command it's called fieldLinkId. Shouldn't we align these as well? I feel like it should be called id with short i.

@milanholemans
Copy link
Contributor

Would also be a nice enhancement to specify a field by its internal name.

@Jwaegebaert
Copy link
Contributor Author

fieldLinkId should indeed better become id. So then for both commands, we would go for -i, --id <id> as the ID of the field. Then for spo contenttype field remove we'll need to remove the short option for contentTypeId as well.

I think it would also be better that we don't add any short option to contentTypeId for clarity.

Would also be a nice enhancement to specify a field by its internal name.

Sounds like a good idea. The field's internal name is much more used than its ID. Maybe something we open up a separate issue for?

@milanholemans
Copy link
Contributor

fieldLinkId should indeed better become id. So then for both commands, we would go for -i, --id <id> as the ID of the field. Then for spo contenttype field remove we'll need to remove the short option for contentTypeId as well.

I think it would also be better that we don't add any short option to contentTypeId for clarity.

Would also be a nice enhancement to specify a field by its internal name.

Sounds like a good idea. The field's internal name is much more used than its ID. Maybe something we open up a separate issue for?

I agree with everything 🙂

@Jwaegebaert
Copy link
Contributor Author

The specs are updated

@Jwaegebaert Jwaegebaert added help wanted and removed needs peer review Needs second pair of eyes to review the spec or PR labels Jul 23, 2024
@MathijsVerbeeck MathijsVerbeeck self-assigned this Sep 4, 2024
ktskumar pushed a commit to ktskumar/cli-microsoft365 that referenced this issue Oct 8, 2024
ktskumar pushed a commit to ktskumar/cli-microsoft365 that referenced this issue Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment