SqlTraceFlag: New resource #1640

Fiander · 2020-11-26T17:56:27Z

Pull Request (PR) description

New Resource SqlTraceFlag.

This Pull Request (PR) fixes the following issues

Task list

Added an entry to the change log under the Unreleased section of the
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
Resource documentation updated in the resource's README.md.
Resource parameter descriptions updated in schema.mof.
Comment-based help updated, including parameter descriptions.
Localization strings updated.
Examples updated.
Unit tests updated. See DSC Community Testing Guidelines.
Integration tests updated (where possible). See DSC Community Testing Guidelines.
Code changes adheres to DSC Community Style Guidelines.

This change is

codecov · 2020-11-27T02:16:58Z

Codecov Report

Merging #1640 (281d19b) into master (c7146f9) will increase coverage by 0%.
The diff coverage is 99%.

@@           Coverage Diff           @@
##           master   #1640    +/-   ##
=======================================
  Coverage      98%     98%            
=======================================
  Files          37      38     +1     
  Lines        6040    6177   +137     
=======================================
+ Hits         5949    6085   +136     
- Misses         91      92     +1

Flag	Coverage Δ
unit	`98% <99%> (+<1%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...SCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1	`99% <99%> (ø)`
source/DSCResources/DSC_SqlSetup/DSC_SqlSetup.psm1	`98% <100%> (ø)`
...SCResources/DSC_SqlWaitForAG/DSC_SqlWaitForAG.psm1	`100% <100%> (ø)`

Fiander · 2020-11-27T07:52:02Z

Small question.
for the big build i had to change in the schema.mof:
[Write...] Uint32[] TraceFlags;
into
[Write...] Uint32 TraceFlags[];

Why did it build correctly at home?

johlju · 2020-11-27T07:59:21Z

Did you run the HQRM test at home? One test it does is to parse the schema mof's.

.\build.ps1 -Tasks hqrmtest

It is a separate test because it takes a while to run and does not work cross-platform.

Fiander · 2020-11-27T08:22:48Z

That is a test i can not run. ALL resource get this message:
Multiple versions of the module 'SqlServerDsc' were found. You can run 'Get-Module -ListAvailable -FullyQualifiedName SqlServerDsc' to see available versions on the system
, and then use the fully qualified name '@{ModuleName="SqlServerDsc"; RequiredVersion="Version"}'.

so i think i need to remove the normal module, or move all develop work ( of halve of it ) into a sepperate VM.

Will look into that. Thank you for the suggestion about that test.

johlju · 2020-11-27T08:35:39Z

Yes, that might be a problem. I don't have any modules I develop in any PSModulePath because of that.

johlju

Reviewed 2 of 10 files at r1.
Reviewable status: 2 of 10 files reviewed, 12 unresolved discussions (waiting on @Fiander)

a discussion (no related file):
I sent in a PR Fiander#1 for style fixes instead of comment on all of them. 🙂

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 68 at r1 (raw file):

return @{

This should return all properties in the schema.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 164 at r1 (raw file):

$TraceFlags

Can we use $PSBoundParameters.ContainsKey('TraceFlags') when checking if the parameter was set as part of the configuration and should be enforced? We should be able to pass @() to remove all trace flags.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 173 at r1 (raw file):

$TraceFlagsToInclude

Can we use $PSBoundParameters.ContainsKey('TraceFlagsToInclude') when checking if the parameter was set as part of the configuration and should be enforced?

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 184 at r1 (raw file):

$TraceFlagsToExclude

Can we use $PSBoundParameters.ContainsKey('TraceFlagsToExclude') when checking if the parameter was set as part of the configuration and should be enforced?

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 353 at r1 (raw file):

if ($TraceFlags)

Can we use $PSBoundParameters.ContainsKey('TraceFlags') when checking if the parameter was set as part of the configuration and should be enforced?

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 355 at r1 (raw file):

Compare-Object -ReferenceObject $getTargetResourceResult.ActualTraceFlags -DifferenceObject $TraceFlags

Can we move this out of the evaluation and set it to a variable that is used in evaluation instead, to make the code a bit more readable.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 367 at r1 (raw file):

if ($TraceFlagsToInclude)

Can we use $PSBoundParameters.ContainsKey('TraceFlagsToInclude') when checking if the parameter was set as part of the configuration and should be enforced?

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 383 at r1 (raw file):

$TraceFlagsToExclude

Can we use $PSBoundParameters.ContainsKey('TraceFlagsToExclude') when checking if the parameter was set as part of the configuration and should be enforced?

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 410 at r1 (raw file):

Export-ModuleMember -Function *-TargetResource

We could actually remove this as it is not necessary. Old inheritance.
The resource SqlProtocol and SqlProtocolTcpIp does not have this. Haven't gotten around to remove it from all other resources.
If you want to keep it, move it to the end of the file. 🙂

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 416 at r1 (raw file):

Get-SqlServiceName

Non-blocking (Informational): This is similar to this, but not quite, seems this should be made into a common function in the future.

SqlServerDsc/source/DSCResources/DSC_SqlServiceAccount/DSC_SqlServiceAccount.psm1

Line 514 in a235aa3

function Get-SqlServiceName

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 420 at r1 (raw file):

Position = 1

We can remove this as we always should use named parameters.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 422 at r1 (raw file):

$InstanceName

Can we add this to the comment-based help.

johlju

Reviewed 1 of 10 files at r1.
Reviewable status: 3 of 10 files reviewed, 13 unresolved discussions (waiting on @Fiander)

tests/Unit/Stubs/SMO.cs, line 975 at r1 (raw file):

.ManagedServiceType

Change to .ManagedComputer

Fiander

Reviewable status: 1 of 10 files reviewed, 13 unresolved discussions (waiting on @Fiander and @johlju)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 68 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

return @{
This should return all properties in the schema.

Done, But seems to me like a wast of resources. ( not your fault!)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 164 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$TraceFlags
Can we use $PSBoundParameters.ContainsKey('TraceFlags') when checking if the parameter was set as part of the configuration and should be enforced? We should be able to pass @() to remove all trace flags.

Done, but isnt removing done by setting the resource to Absent?

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 173 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$TraceFlagsToInclude
Can we use $PSBoundParameters.ContainsKey('TraceFlagsToInclude') when checking if the parameter was set as part of the configuration and should be enforced?

Done.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 184 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$TraceFlagsToExclude
Can we use $PSBoundParameters.ContainsKey('TraceFlagsToExclude') when checking if the parameter was set as part of the configuration and should be enforced?

Done.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 353 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

if ($TraceFlags)
Can we use $PSBoundParameters.ContainsKey('TraceFlags') when checking if the parameter was set as part of the configuration and should be enforced?

Done.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 355 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Compare-Object -ReferenceObject $getTargetResourceResult.ActualTraceFlags -DifferenceObject $TraceFlags
Can we move this out of the evaluation and set it to a variable that is used in evaluation instead, to make the code a bit more readable.

Done.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 367 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

if ($TraceFlagsToInclude)
Can we use $PSBoundParameters.ContainsKey('TraceFlagsToInclude') when checking if the parameter was set as part of the configuration and should be enforced?

Done.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 383 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$TraceFlagsToExclude
Can we use $PSBoundParameters.ContainsKey('TraceFlagsToExclude') when checking if the parameter was set as part of the configuration and should be enforced?

Done.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 410 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Export-ModuleMember -Function *-TargetResource
We could actually remove this as it is not necessary. Old inheritance.
The resource SqlProtocol and SqlProtocolTcpIp does not have this. Haven't gotten around to remove it from all other resources.
If you want to keep it, move it to the end of the file. 🙂

Done. removed it.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 416 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Get-SqlServiceName
Non-blocking (Informational): This is similar to this, but not quite, seems this should be made into a common function in the future.

SqlServerDsc/source/DSCResources/DSC_SqlServiceAccount/DSC_SqlServiceAccount.psm1

Line 514 in a235aa3

function Get-SqlServiceName

i agree.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 420 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Position = 1
We can remove this as we always should use named parameters.

Done.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 422 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$InstanceName
Can we add this to the comment-based help.

Done.

tests/Unit/Stubs/SMO.cs, line 975 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

.ManagedServiceType
Change to .ManagedComputer

Oops.... stupid one..... Done.

Fiander

Reviewable status: 1 of 10 files reviewed, 13 unresolved discussions (waiting on @Fiander and @johlju)

tests/Unit/Stubs/SMO.cs, line 975 at r1 (raw file):

Previously, Fiander wrote…

Oops.... stupid one..... Done.

Done.

johlju · 2020-11-27T17:48:37Z

If you can look at the PR Fiander#1 in your fork and merge it. 🙂

Style guideline fixes and move change log entries

Fiander · 2020-11-27T18:09:43Z

thank you, i didnot see that one

johlju

Reviewed 2 of 5 files at r3.
Reviewable status: 2 of 10 files reviewed, 3 unresolved discussions (waiting on @Fiander and @johlju)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 164 at r1 (raw file):

Previously, Fiander wrote…

Done, but isnt removing done by setting the resource to Absent?

Actually, in this resource there is no point of having the Ensure property since it does no create or remove an object. It just changes a setting, so setting the TraceFlags to @() should be the right way to enforce that there should be no trace flags.

Fiander

Reviewable status: 2 of 10 files reviewed, 3 unresolved discussions (waiting on @Fiander and @johlju)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 164 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Actually, in this resource there is no point of having the Ensure property since it does no create or remove an object. It just changes a setting, so setting the TraceFlags to @() should be the right way to enforce that there should be no trace flags.

oke. But then we should also remove the ensure parameter.

johlju

Reviewable status: 2 of 10 files reviewed, 3 unresolved discussions (waiting on @Fiander and @johlju)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 164 at r1 (raw file):

Previously, Fiander wrote…

oke. But then we should also remove the ensure parameter.

Yes, sorry I meant that. The ensure property in the scheme and the parameter in the code. 🙂

PlagueHO · 2020-12-03T06:24:27Z

I understand the reasoning here, but we should probably base the pattern around what the PowerShell team have done with the Group resource in PSDesiredStateConfiguration and PSDscResources - as these are the "in-box" resources (and replacement for the "in-box" resources). They are the most used resources across DSC, so that pattern is most understood/expected.

It appears that this resource is implementing the same pattern, where the actual value is returned in the Members property and the MembersToInclude and MembersToExclude are returned as $null.

Not saying that is the perfect pattern by any means 😁 just that it is probably most understood because of it's wide use.

…all test for this.

johlju · 2020-12-04T18:00:41Z

Would moving the mock code to a different file be an idea?

I rather not because I think that would make it harder for newcomers to understand that dependent code is in another file.
Our goal should be having tests that are easily to read and easy to follow. We should avoid having contributors that are new to Pester to need to jump up and down in the file, that includes not needing coding in two files.

For integration tests I think the configuration is in another file to look more like a real configuration file, and it is meant that the config file could get input from a third file (real world properties). But I want these to be simple as well so making these into one file too would make them much easier to read as well, but we don't have to go there now with the integ tests. 🙂

johlju

Reviewed 3 of 6 files at r5, 1 of 4 files at r6.
Reviewable status: 7 of 11 files reviewed, 4 unresolved discussions (waiting on @Fiander and @johlju)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 320 at r4 (raw file):

Previously, Fiander wrote…

Done.

I think you missed this one? 🙂

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 45 at r6 (raw file):

e  = $

We can remove the extra space before the equal sign.

tests/Unit/Stubs/New File.ps1, line 0 at r6 ([raw file](https://github.com/dsccommunity/sqlserverdsc/blob/ce786852a984ea364e1213d936f647bc5ebe4a30/tests/Unit/Stubs/New File.ps1#L0)):
Seems we got an extra empty file here.

johlju · 2020-12-04T18:10:29Z

It was a bit busy at the day job this week, but resolved all the comments that were done. Awesome work on this, soon done! 😃

Fiander

Reviewable status: 7 of 11 files reviewed, 4 unresolved discussions (waiting on @johlju)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 320 at r4 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

I think you missed this one? 🙂

yes i did......

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 45 at r6 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

e  = $
We can remove the extra space before the equal sign.

Done.

tests/Unit/DSC_SqlTraceFlag.Tests.ps1, line 113 at r4 (raw file):

Previously, Fiander wrote…

Will do, but that wil not be now. i have a familimeeting to attend to. Back in a few hours.

done.

tests/Unit/Stubs/New File.ps1, line at r6 ([raw file](https://github.com/dsccommunity/sqlserverdsc/blob/ce786852a984ea364e1213d936f647bc5ebe4a30/tests/Unit/Stubs/New File.ps1#L)):

Get-function.
Where did that one come from? removed it

Fiander · 2020-12-04T19:17:12Z

It was a bit busy at the day job this week, but resolved all the comments that were done. Awesome work on this, soon done! 😃

No problem, i finally have my branches in working order :-)

when do you want to go to version 15?
because i am also working on a fix for #1492
https://github.com/Fiander/SqlServerDsc/tree/SqlAgDatabase.

now finishing up on the tests i hope. I think i fixed all i did not good in 1640, so that one should be easieer for you to review

sry about this one, but i want to improve, and that means make mistakes/misinterpretations.

johlju · 2020-12-05T10:02:27Z

when do you want to go to version 15?

We in the community decide. I usually try to get as many breaking changes into a breaking change release as possible, but I haven't gotten the time to help out creating PR's at the moment. So if there are no more breaking changes in the works then we could release anytime. 🙂

sry about this one, but i want to improve, and that means make mistakes/misinterpretations.

No need for sorry! The community, and especially I, really appreciate the time you put in to these PRs! 🙇‍♂️ The discussions we have is natural and a good thing, and I appreciate to get new perspectives from you! 🙂
You do quality work! Do not feel like you have make any mistakes - it is natural that there are discussions/comments during the review process. 😃

I review the last bits now.

johlju

Reviewed 1 of 6 files at r5, 3 of 3 files at r7.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Fiander)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 244 at r7 (raw file):

$RestartTimeout = 10

Could you make an issue for this so we can fix this in another PR. I think we should make this as parameter in the future because 10 seconds can be slow on some machines. A parameter similar to this:

SqlServerDsc/source/DSCResources/DSC_SqlConfiguration/DSC_SqlConfiguration.schema.mof

Line 9 in ce78685

    
           [Write, Description("The length of time, in seconds, to wait for the service to restart. Default is `120` seconds.")] UInt32 RestartTimeout;

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 354 at r7 (raw file):

$ServerRoleName

This one should be removed I think, because the localized string has no placeholder for any variables.

But we could also change the localized string to output the actual and desired trace flags, maybe The trace flags does not match. Expected '{0}', but was '{1}'.. Then join the array to a string to pass to the localized string, e.g$TraceFlags -join ','

I let you decide. 🙂

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.schema.mof, line 9 at r7 (raw file):

RestartInstance

In other resources we have named this RestartService, is it worth renaming to that here to? I good with either way. I let you decide.

tests/Unit/DSC_SqlTraceFlag.Tests.ps1, line 824 at r7 (raw file):

{
    Invoke-TestCleanup
}

Can we make a empty line at the end in this file. I usually set the code editor to make this automatically on save (like VS Code).
Reviewable is indicating if there are no new line at the end of a file. 🙂

Fiander

Reviewable status: 6 of 10 files reviewed, 3 unresolved discussions (waiting on @johlju)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 348 at r4 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

ActualTraceFlags
This should be changed to TraceFlags. See comment in Get-function. Throughout the Test-function.

hmm, did these yesterday, but they were undone? done them again.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 244 at r7 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$RestartTimeout = 10
Could you make an issue for this so we can fix this in another PR. I think we should make this as parameter in the future because 10 seconds can be slow on some machines. A parameter similar to this:

SqlServerDsc/source/DSCResources/DSC_SqlConfiguration/DSC_SqlConfiguration.schema.mof

Line 9 in ce78685

[Write, Description("The length of time, in seconds, to wait for the service to restart. Default is `120` seconds.")] UInt32 RestartTimeout;

In that case, i would rather do that now.
Adding this in the scheme, is not much work, and would save a breaking change in the future.
the default can then be 120 seconds or so.

changes in code would be moving that one line up as parameter ( in all functions, and examples )
tests can be speeded up by setting this one to 1 i think
when you say "YES", i will change it now, otherwise i will change the 10 seconds to 120.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 354 at r7 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$ServerRoleName
This one should be removed I think, because the localized string has no placeholder for any variables.

But we could also change the localized string to output the actual and desired trace flags, maybe The trace flags does not match. Expected '{0}', but was '{1}'.. Then join the array to a string to pass to the localized string, e.g$TraceFlags -join ','

I let you decide. 🙂

the long one it is.
When using dsc-resources i like it when they tell me what went wrong, including the values.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.schema.mof, line 9 at r7 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

RestartInstance
In other resources we have named this RestartService, is it worth renaming to that here to? I good with either way. I let you decide.

no problem. : my logic >> i want the instance to restart, so restartInstance. but changed it to match all others.

tests/Unit/DSC_SqlTraceFlag.Tests.ps1, line 824 at r7 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Can we make a empty line at the end in this file. I usually set the code editor to make this automatically on save (like VS Code).
Reviewable is indicating if there are no new line at the end of a file. 🙂

Done.

Fiander

Reviewable status: 6 of 10 files reviewed, 2 unresolved discussions (waiting on @johlju)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 354 at r7 (raw file):

Previously, Fiander wrote…

the long one it is.
When using dsc-resources i like it when they tell me what went wrong, including the values.

Done.

johlju

Reviewable status: 6 of 10 files reviewed, 2 unresolved discussions (waiting on @johlju)

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 244 at r7 (raw file):

Previously, Fiander wrote…

In that case, i would rather do that now.
Adding this in the scheme, is not much work, and would save a breaking change in the future.
the default can then be 120 seconds or so.

changes in code would be moving that one line up as parameter ( in all functions, and examples )
tests can be speeded up by setting this one to 1 i think
when you say "YES", i will change it now, otherwise i will change the 10 seconds to 120.

Yes, you are welcome to add it now. I thought I'd spare you yet another change. 😉 But let's add it now if you are up to it.

johlju

This looks good to me now. Waiting for you to add the RestartTimeout parameter since it sounded you were up to it. 🙂

Reviewed 4 of 4 files at r8.
Reviewable status: complete! all files reviewed, all discussions resolved

johlju · 2020-12-05T11:48:01Z

@Fiander HQRM test failed on the examples. Parameter RestartInstance need to be renamed there as well. 🙂

johlju

Reviewable status: 4 of 10 files reviewed, 2 unresolved discussions (waiting on @Fiander and @johlju)

a discussion (no related file):
Can you update the list of resources here as well?

SqlServerDsc/source/SqlServerDsc.psd1

Line 38 in ce78685

DscResourcesToExport = @(

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 68 at r9 (raw file):

return @{

Now we need the RestartTimeout here to. 😉

Fiander

And i missed one in a unit test.

Reviewable status: 4 of 11 files reviewed, 2 unresolved discussions (waiting on @johlju)

a discussion (no related file):

Previously, johlju (Johan Ljunggren) wrote…

Can you update the list of resources here as well?

SqlServerDsc/source/SqlServerDsc.psd1

Line 38 in ce78685

DscResourcesToExport = @(

yes i can, and i did

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 244 at r7 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Yes, you are welcome to add it now. I thought I'd spare you yet another change. 😉 But let's add it now if you are up to it.

already done. waiting for hqrm to succeed

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 68 at r9 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

return @{
Now we need the RestartTimeout here to. 😉

damm it...

Fiander

Reviewable status: 4 of 11 files reviewed, 2 unresolved discussions (waiting on @johlju)

a discussion (no related file):

Previously, Fiander wrote…

yes i can, and i did

Done.

source/DSCResources/DSC_SqlTraceFlag/DSC_SqlTraceFlag.psm1, line 68 at r9 (raw file):

Previously, Fiander wrote…

damm it...

Done.

johlju

Reviewed 5 of 6 files at r9, 2 of 2 files at r10.
Reviewable status: complete! all files reviewed, all discussions resolved

johlju · 2020-12-05T20:43:16Z

Waiting for the tests to pass and then we can merge this. 😃

johlju · 2020-12-06T08:31:05Z

@Fiander awesome work on this! 😄 I can release a new full version of SqlServerDsc later today.

Fiander added 5 commits November 26, 2020 18:51

Initial version of new resource SqlTraceFlag

69ebf57

small change to trigger pipline

e1cc2da

Change Schema friendliname

f7cab74

schema change

105dd1b

some HQRM fixes

c5346a2

johlju added the needs review The pull request needs a code review. label Nov 27, 2020

johlju mentioned this pull request Nov 27, 2020

Style guideline fixes and move change log entries Fiander/SqlServerDsc#1

Merged

9 tasks

johlju requested changes Nov 27, 2020

View reviewed changes

johlju added waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed needs review The pull request needs a code review. labels Nov 27, 2020

first batch of fixes

8125732

Fiander commented Nov 27, 2020

View reviewed changes

Fiander and others added 3 commits November 27, 2020 17:53

Removed to much

5ff9a87

Style guideline fixes and move change log entries

7e92280

Fix examples

a7fbcf3

Merge pull request #1 from johlju/traceflag-styleguideline

e8f27db

Style guideline fixes and move change log entries

johlju mentioned this pull request Nov 27, 2020

New resource: SqlDebug #562

Open

johlju requested changes Nov 27, 2020

View reviewed changes

johlju self-requested a review November 27, 2020 19:26

Fiander commented Nov 27, 2020

View reviewed changes

johlju requested changes Nov 27, 2020

View reviewed changes

removed readonly parameters, changed to Restart-SqlService, adjusted …

ca65655

…all test for this.

johlju requested changes Dec 4, 2020

View reviewed changes

johlju self-requested a review December 4, 2020 18:08

last fixes ( i hope )

36eb33c

Fiander commented Dec 4, 2020

View reviewed changes

johlju requested changes Dec 5, 2020

View reviewed changes

last fixes

8b603c3

Fiander commented Dec 5, 2020

View reviewed changes

johlju reviewed Dec 5, 2020

View reviewed changes

johlju self-requested a review December 5, 2020 11:39

johlju approved these changes Dec 5, 2020

View reviewed changes

Fiander added 2 commits December 5, 2020 13:50

Last changes. ( i hope ;-) )

ba5ff81

mised a RestartService

bd76c22

johlju requested changes Dec 5, 2020

View reviewed changes

johlju self-requested a review December 5, 2020 17:46

Adden to SqlServerDsc and SqlTraceFlag get missed a return value

281d19b

Fiander commented Dec 5, 2020

View reviewed changes

johlju approved these changes Dec 5, 2020

View reviewed changes

johlju merged commit f5724c7 into dsccommunity:master Dec 6, 2020

johlju removed the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Dec 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SqlTraceFlag: New resource #1640

SqlTraceFlag: New resource #1640

Fiander commented Nov 26, 2020 •

edited by johlju

Loading

codecov bot commented Nov 27, 2020 •

edited

Loading

Fiander commented Nov 27, 2020

johlju commented Nov 27, 2020

Fiander commented Nov 27, 2020

johlju commented Nov 27, 2020

johlju left a comment

johlju left a comment

Fiander left a comment

Fiander left a comment

johlju commented Nov 27, 2020

Fiander commented Nov 27, 2020

johlju left a comment

Fiander left a comment

johlju left a comment

PlagueHO commented Dec 3, 2020

johlju commented Dec 4, 2020

johlju left a comment

johlju commented Dec 4, 2020

Fiander left a comment

Fiander commented Dec 4, 2020

johlju commented Dec 5, 2020

johlju left a comment

Fiander left a comment

Fiander left a comment

johlju left a comment

johlju left a comment

johlju commented Dec 5, 2020

johlju left a comment

Fiander left a comment

Fiander left a comment

johlju left a comment

johlju commented Dec 5, 2020

johlju commented Dec 6, 2020

SqlTraceFlag: New resource #1640

SqlTraceFlag: New resource #1640

Conversation

Fiander commented Nov 26, 2020 • edited by johlju Loading

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Task list

codecov bot commented Nov 27, 2020 • edited Loading

Codecov Report

Fiander commented Nov 27, 2020

johlju commented Nov 27, 2020

Fiander commented Nov 27, 2020

johlju commented Nov 27, 2020

johlju left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

Fiander left a comment

Choose a reason for hiding this comment

Fiander left a comment

Choose a reason for hiding this comment

johlju commented Nov 27, 2020

Fiander commented Nov 27, 2020

johlju left a comment

Choose a reason for hiding this comment

Fiander left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

PlagueHO commented Dec 3, 2020

johlju commented Dec 4, 2020

johlju left a comment

Choose a reason for hiding this comment

johlju commented Dec 4, 2020

Fiander left a comment

Choose a reason for hiding this comment

Fiander commented Dec 4, 2020

johlju commented Dec 5, 2020

johlju left a comment

Choose a reason for hiding this comment

Fiander left a comment

Choose a reason for hiding this comment

Fiander left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

johlju commented Dec 5, 2020

johlju left a comment

Choose a reason for hiding this comment

Fiander left a comment

Choose a reason for hiding this comment

Fiander left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

johlju commented Dec 5, 2020

johlju commented Dec 6, 2020

Fiander commented Nov 26, 2020 •

edited by johlju

Loading

codecov bot commented Nov 27, 2020 •

edited

Loading