dsccommunity / SqlServerDsc

This module contains DSC resources for deployment and configuration of Microsoft SQL Server.
MIT License
360 stars 227 forks source link

`Get-SqlDscAudit`: Should not enforce to use parameter `Name` #1812

Closed johlju closed 1 year ago

johlju commented 2 years ago

Problem description

When running the command Get-SqlDscAudit without Name the command ask for a value for Name instead of returning all audits.

Verbose logs

PS > $serverObject | Get-SqlDscAudit

cmdlet Get-SqlDscAudit at command pipeline position 1
Supply values for the following parameters:
Name:

How to reproduce

Run Get-SqlDscAudit without Name parameter.

Expected behavior

Return all available audits.

Current behavior

Stops and asks for a single audit name.

Suggested solution

The parameter Name should not be mandatory.

Operating system the target node is running

Any.

PowerShell version and build the target node is running

n/a

Module version used

Main branch.
bleakprestiger commented 2 years ago

Can I Take This Issue ? I am interested into looking into this issue.

johlju commented 2 years ago

You are so welcome, looking forward to review a PR. 🙂

bleakprestiger commented 2 years ago

Can you assign this issue to me.

johlju commented 2 years ago

Thought I could not assign other than members but I could. This is now assigned to you.

bleakprestiger commented 2 years ago

i had a minor doubt, so for creating audits what we could do ? for e.g. in order to test this Get-SqlDscAudit in powershell, we would have to create some audits, so how could we create the same ?

johlju commented 2 years ago

For this you only need to make a unit test to test the code change. In a unit test you just mock the audits with "mocking code".

Take a look at the existing unit test and re-use the same principle to add a new Context- and It-block as necessary.

bleakprestiger commented 1 year ago

On analyzing the issue, it seems that the parameter Name is being used as a mandatory parameter, as it is being mentioned by the Mandatory Annotation.

johlju commented 1 year ago

Yes. But it should not need to be mandatory, I think it should instead be optional.

johlju commented 1 year ago

@bleakprestiger are you still on this issue?

bleakprestiger commented 1 year ago

Yes

johlju commented 1 year ago

@bleakprestiger I have not seen and PR for this for over a month. I will remove the assignment within a week unless I see a PR, so someone else can work on this.