Adds spp autofillcolumn apply command. Closes #6203#6460
Conversation
2ff3284 to
50c5529
Compare
|
Thank you, we'll try to review it ASAP! |
|
Sorry for the late reply @mkm17, I totally missed your remark last time.
Is this something we are using at a lot of other spots? Seems like in your code you are only retrieving the list
SPO util will be the right place I guess, yes.
To me, it kind of depends on how many places we'll have to replace code by this util function. If it's just one other command, you can easily add it to this PR. If there are like 5 other commands sharing the same logic, I'd opt for a separate issue/PR. |
064b2f6 to
db18aae
Compare
|
Hi, according to the latest comments from @martinlingstuyl on the specification, I have changed the current apply command to set. The I have used the column property I noticed an issue with the test stage in the PR checks, but all tests pass when I run them locally. Can we rerun these checks without changing the code? |
I just did rerun them, but it seems like it doesn't help. I think something is wrong with the macOs pipeline, in the last couple of days I saw this pipeline failing on other PRs as well. We'll have to take a look at it. |
db18aae to
d476d53
Compare
|
Hi @mkm17, I rebased your PR to the latest main to see if our pipeline has been fixed. I hope you don't mind if we take you as a test. Since I performed a force push, I recommend you delete your local branch and pull again from your forked repo. |
779edbc to
81f60aa
Compare
|
Hi @milanholemans sorry for the delay. I've finally made some changes to this command. it's now ready for review. |
martinlingstuyl
left a comment
There was a problem hiding this comment.
Hi @mkm17, I've reviewed your PR, looks like solid work. I do have some comments. Could you look into those and fix them?
|
Oh and also could you fix/rebase the branch to fix the merge conflicts? Thanks in advance! |
81f60aa to
c5ddbc6
Compare
c5ddbc6 to
346b519
Compare
|
@martinlingstuyl thank you for the review. I hope that I have covered all the suggestions. |
|
@martinlingstuyl are you planning to push this PR forward? |
|
Hi @Adam-it, I'm unable to pick it up currently, unfortunately... would you pick it up for me? |
|
On it |
Adds spp autofillcolumn apply command. Closes #6203
Hi, just a few remarks on this command:
I’ve added the
columnIdandcolumnTitleparameters to allow selection of the column by either option.I noticed that retrieving the list instance, like in the
getDocumentLibraryInfofunction, appears in several places in the solution. Could you advise on where we should store this common code? Is spo.ts the correct file for it?Similarly, for retrieving a field, should the
getColumnfunction also be placed in spo.ts?Do you think I should make these updates in this PR, or would it be better to create a separate one?