search

MethodsAndPractices / vsteam

PowerShell module for accessing Azure DevOps Services and Azure DevOps Server (formerly VSTS or TFS)
https://methodsandpractices.github.io/vsteam-docs/
MIT License
445 stars 155 forks source link

Fix two annoying behaviors in new completers and validators #299

Closed jhoneill closed 4 years ago

jhoneill commented 4 years ago

When I made the changes to remove dynamic parameters ( included in #289) I missed two things.

  1. A mandatory parameters will enforce Not-Null-or-Empty , unless an Allow is specified and for a non-mandatory parameter this should be enforced by a validate attribute. The validators I wrote fail if null or empty is passed and should not do so.

  2. Completers which may return items with spaces (or any 'non-word' character which might need wrapping in quotes) should wrap the item in quotes, doubling up any quote marks in the item.

I have already done the work for this :-) but I wanted an issue to link an upcomming PR to.

SebastianSchuetze commented 4 years ago

I will link the PR as soon as you submitted it!

jhoneill commented 4 years ago

@SebastianSchuetze I have been working on something in the day job which meant I wrote about 20 new commands for working with processes, and the work item types, and then fields / pages / groups / controls. This one , #300 and #301 are pre-requisites for that work, so I'm trying to make them one manageable PR, and then the others will come in their own PR. So I have been splitting the work into two parts. I need to some more checks before putting in the PR but it should be this weekend .

SebastianSchuetze commented 4 years ago

Very Cool! But it is fine if the PRs are smaller rather than big. Check #269 for a discussion about cmdlet. Currently we are only wrapping the API and not concentrating on higher level cmdlet which are probably something for another module. But we are not sure yet.

jhoneill commented 4 years ago

These only wrap the API. My feeling the higher level things, FWIW is that often those are better as example scripts than as a part of the core module. So "Oh, you want to take export Pipeline specifications to an XLSx file, edit them and import them back - here's an example" not "Lets add Export-PipelineToXLSX and Import-PipelineFromXLSX" as functions.

DarqueWarrior commented 4 years ago

Just make sure you merge all the changes from master. A ton of work was done to move the original changes over. Not all the classes were exactly the same as you wrote them originally. Just want to make sure we don't lose any changes.

jhoneill commented 4 years ago

I've merged everything from master, and found your changes, and incorporated them / made new stuff match them. Sebastian also pointed out some conflicts of style and naming and I'm trying to follow those too. Contributing to multiple projects with different people's ways of doing things is like switching accents to talk to different groups of people :-)