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

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

Closed
johlju opened this issue Nov 29, 2022 · 11 comments · Fixed by #1850
Closed

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

johlju opened this issue Nov 29, 2022 · 11 comments · Fixed by #1850
Labels
enhancement The issue is an enhancement request. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub

Comments

@johlju
Copy link
Member

johlju commented Nov 29, 2022

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.
@johlju johlju added enhancement The issue is an enhancement request. help wanted The issue is up for grabs for anyone in the community. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub labels Nov 29, 2022
@bleakprestiger
Copy link
Contributor

bleakprestiger commented Nov 30, 2022

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

@johlju
Copy link
Member Author

johlju commented Nov 30, 2022

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

@bleakprestiger
Copy link
Contributor

Can you assign this issue to me.

@johlju
Copy link
Member Author

johlju commented Nov 30, 2022

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

@bleakprestiger
Copy link
Contributor

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
Copy link
Member Author

johlju commented Nov 30, 2022

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
Copy link
Contributor

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
Copy link
Member Author

johlju commented Dec 4, 2022

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

@johlju
Copy link
Member Author

johlju commented Dec 18, 2022

@bleakprestiger are you still on this issue?

@bleakprestiger
Copy link
Contributor

Yes

@johlju
Copy link
Member Author

johlju commented Jan 22, 2023

@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.

@johlju johlju added in progress The issue is being actively worked on by someone. and removed help wanted The issue is up for grabs for anyone in the community. labels Feb 19, 2023
@johlju johlju removed the in progress The issue is being actively worked on by someone. label Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is an enhancement request. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants