Open mkm17 opened 4 months ago
@mkm17 awesome work 👏👏 You Rock 🤩
I have a few comments we could consider:
viewOption
? if it's only NewViewAsDefault
I would suggest we specify this as flag option in our command and then we should specify it like this in spec --viewOption
so withouth [viewOption]
title
and specify to use ether title
or id
id
instead of modelUniqueId
. First of all since the command is spo stx model apply
then the model
word does not need to be duplicated in the options. Also id
will be more aligned with what is typically used in CLI alreadylibraryUrl
I don't quite feel this naming. Usually we use list...
but I get it that we should only support libs not lists... hmm usually in such a case we have like folderRelativeUrl
or something similar.. but I man sure how to correct this naming here🤔@pnp/cli-for-microsoft-365-maintainers any other feed? or suggestion on my last point 👆
@Adam-it ok, I have removed siteUrl and added folderUrl. It is a similar approach to the file add command. The rest of your comments have been applied.
Another great job @mkm17!
I also would suggest removing the default properties
. This won't be a list command so it isn't necessary here to specify them.
Regarding --viewOption
, I'm not quite sure about the name for this option as it sounds a bit vague compared to the description. Maybe something like --viewDefault
or --defaultModel
.
I would also opt for --folderUrl
, which looks clear enough.
@Jwaegebaert what about --newDefaultView
as a name of the option?
To me folderUrl
isn't that clear. It looks like I can specify any folder in a list which isn't the case right?
Why not try to be consistent and use listTitle
, listUrl
, listId
? Even if you can only specify doc libraries, I think we can still use these options?
@Jwaegebaert what about
--newDefaultView
as a name of the option?
I find the new
part a bit vague and feel it doesn't add much meaning to what it does, but on the other hand, defaultView
seems like a good way to go about it.
Why not try to be consistent and use
listTitle
,listUrl
,listId
? Even if you can only specify doc libraries, I think we can still use these options?
It seems like we aren't very consistent yet. Most commands do use this casing, but it's not everywhere just yet. Still, it's a good idea to apply similar naming here too.
Ok, I have taken the same approach as in the sensitivitylabel
@mkm17 awesome work. Lets open it up 👍
Ok, I can take also this one ;)
Hey @mkm17, thank you for all the hard work you’ve put in, it’s really appreciated! We’ve noticed that you have several issues assigned that haven't yet been created with a PR. To ensure everyone has a chance to get involved, especially with Hacktoberfest, I’ll be unassigning you from some of these issues.
We recommend focusing on 1 to 2 issues at a time. Once you’ve submitted a PR for an issue, you can easily claim another open one. This approach helps you manage the workload more effectively and ensures you can give each task the attention it needs.
Thanks again for your continued contributions!
@Jwaegebaert Omg, time passed so quickly. Yes, definitely let's open it to everyone. Thanks!
@Jwaegebaert the same with this one, can I take it?
Usage
m365 spp model apply [options]
Description
Applies (or syncs) a trained document understanding model to a document library.
Options
-u, --webUrl <webUrl>
-i, --id [id]
id
ortitle
but not both.-t, --title [title]
id
ortitle
but not both.--listTitle [listTitle]
listTitle
,listId
, orlistUrl
but not multiple.--listId [listId]
listTitle
,listId
, orlistUrl
but not multiple.--listUrl [listUrl]
listTitle
,listId
, orlistUrl
but not multiple.--defaultView
Examples
Applies a trained document understanding model by id to a document library based on the list title.
Applies a trained document understanding model by title to a document library based on the list title.
Applies a trained document understanding model by title to a document library based on the list url.
Applies a trained document understanding model by title to a document library based on the list id.
Additional Info
This endpoint can be used: https://learn.microsoft.com/en-us/sharepoint/dev/apis/syntex/rest-applymodel-method