20:00:00 <nitzmahone> #startmeeting Ansible Windows Working Group
20:00:00 <zodbot> Meeting started Tue Nov 23 20:00:00 2021 UTC.
20:00:00 <zodbot> This meeting is logged and archived in a public location.
20:00:00 <zodbot> The chair is nitzmahone. Information about MeetBot at https://fedoraproject.org/wiki/Zodbot#Meeting_Functions.
20:00:00 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
20:00:00 <zodbot> The meeting name has been set to 'ansible_windows_working_group'
20:00:04 <nitzmahone> bam
20:00:09 <nitzmahone> #chair jborean93
20:00:09 <zodbot> Current chairs: jborean93 nitzmahone
20:00:12 <briantist> 'allo
20:00:17 <nitzmahone> hiya!
20:00:18 <jborean93> yo
20:00:21 <mkletz> hello!
20:00:24 <nitzmahone> #info agenda https://github.com/ansible/community/issues/581
20:00:36 <briantist> welcome back mkletz
20:00:41 <mkletz> thank you!
20:00:49 <nitzmahone> Since the composite DSC PR got merged, I assume we don't need to talk about that anymore?
20:00:56 <jborean93> Yea I mostly covered that with https://github.com/ansible-collections/ansible.windows/pull/313#issuecomment-974910937
20:01:03 <mkletz> yeah we just had the CI ignore the issue
20:01:33 <jborean93> Essentially the way that PowerShell parses that particular file required OMI to generate the MOF (or something). Since this is really a file just for testing we just skipped that from pslint
20:01:33 <mkletz> I had a question about the changelog fragments
20:01:44 <nitzmahone> nice
20:01:46 <nitzmahone> Go for it
20:01:57 <nitzmahone> #topic open floor
20:02:00 <mkletz> So the changes for composite resources are likely going to take a significant refactor
20:02:18 <mkletz> I'd like to keep PRs as small as possible so they're reasonably reviewable
20:02:36 <mkletz> How to reflect just a refactor in the change log?
20:02:53 <mkletz> it didn't seem appropriate to be a feature or a bug
20:02:58 <briantist> I think usually, the changelog is for user-facing stuff, so may not need an entry at all
20:03:01 <briantist> unless changes behavior
20:03:09 <jborean93> agreed
20:03:09 <mkletz> ok
20:03:11 <mkletz> so in https://github.com/ansible-collections/ansible.windows/pull/315
20:03:11 <nitzmahone> yep
20:03:16 <mkletz> I should remove the changelog fragment?
20:03:41 <nitzmahone> Can go either way for that
20:04:10 <mkletz> kk
20:04:11 <nitzmahone> We'll still often include things like that just so people know there might be a destabilizing change to a module, but it's 50/50
20:04:16 <mkletz> got it
20:04:33 <briantist> that particular one I'd remove personally, unless maybe the error messages will  be changed by it
20:04:52 <mkletz> nope I left all the errors the same
20:04:56 <mkletz> I just de-duped the exit logic
20:05:12 <briantist> technically changing error messages could be "breaking" changes even though they aren't treated like it, because we sometimes base behavior and conditionals on module message output
20:05:35 <mkletz> right
20:05:49 * nitzmahone grumbles about a core team discussion just this morning on that topic ;)
20:05:55 <mkletz> lol
20:06:03 <briantist> heh, I presume the relevant xkcd was posted
20:06:18 <briantist> https://xkcd.com/1172/
20:06:36 <mkletz> hahaha
20:06:38 <nitzmahone> indeed it was
20:06:44 <nitzmahone> (well, referenced at least)
20:07:21 <mkletz> I was going to work on 1 more PR before I start refactoring to standardize parameter blocks and quotes. I assume that's the type of change we wouldn't bother with a changelog on?
20:08:37 <nitzmahone> Yeah, so long as it's not likely to cause user-visible changes in behavior, that's usually fine
20:08:45 <jborean93> just as an FYI we try to avoid style changes unless it's something to satisfy pslint
20:09:24 <mkletz> gotcha, I saw some discussion around making the quotes a default rule in PsScriptAnalyzer but if that's the case I will avoid doing so
20:09:59 <mkletz> easy enough to resolve when the linter starts complaining about it
20:10:01 <jborean93> would be interesting to see how well PSSA works with that rule, there are many cases when you would want either a double or single quote so hard to really enforce that
20:10:24 <mkletz> let me see if I can track it down
20:10:27 <briantist> what are the specifics of this rule?
20:10:38 <nitzmahone> sounds like another one of those potentially overly-pedantic rules 😐
20:10:50 <briantist> ^ am thinking the same
20:11:16 <jborean93> I'm find with those rules if 1. there's an automatic formatter to fix violations, 2. it isn't full of edge cases
20:11:25 <nitzmahone> yep
20:11:33 <jborean93> At least with python `'` and `"` are mostly the same
20:11:53 <nitzmahone> If there end up being a ton of ignores/exceptions for legit reasons, that rule's a good candidate to be disabled
20:12:02 <mkletz> https://github.com/PowerShell/PSScriptAnalyzer/pull/1470
20:12:10 <mkletz> looks like it's been an opt in rule for awhile
20:12:58 <mkletz> but yeah the gist of the discussion seems to be there is "technically" a performance difference but it's like more disruptive and pedantic than it's worth
20:13:29 <jborean93> yea I would struggle to justify such a rule in PowerShell personally
20:13:46 <briantist> the main benefit I could see with that rule, is that does have better performance for me, cognitively 😅
20:13:54 <mkletz> lol
20:13:55 <briantist> I feel like I do less mental processing of the string when I see it single-quoted
20:14:08 <mkletz> I agree but I won't die on that hill
20:14:16 <briantist> oh absolutely agree
20:14:33 <jborean93> at least it does have automatic formatting to fix it :) https://github.com/PowerShell/PSScriptAnalyzer/blob/master/Rules/AvoidUsingDoubleQuotesForConstantString.cs#L90
20:14:39 <briantist> I tend to use single quotes nearly everywhere I can unless I'm following convention in existing code, but yeah, not going to go crazy over it
20:14:47 * nitzmahone has to slap hand numerous times every time he switches back to a curly-bracket-scoped language where single quotes don't mean the same thing
20:15:19 <nitzmahone> Heh, I wonder how bug-ridden that auto-formatter is...
20:15:38 <mkletz> I've actually only ever had it break a file once after using it quite a bit over the years
20:15:44 <nitzmahone> (we found a couple "fun" bugs recently in other rules)
20:16:03 <mkletz> The bug that drives me nuts
20:16:18 <mkletz> is that the unused variable detection can't see into child scopes
20:16:24 <jborean93> I'm hoping to try and fix it sometime this week once I finish my current thing
20:16:46 <mkletz> So if you have a parameter that is only used within a where-object or something
20:16:48 <jborean93> nah, it autoformats `@{foo='bar'}` to `@{foo = 'bar' }` (the extra space at the end)
20:16:55 <nitzmahone> You might see some "creative" spacing until they fix it
20:17:06 <nitzmahone> --^ that
20:17:12 <jborean93> https://github.com/PowerShell/PSScriptAnalyzer/issues/1742
20:17:17 <briantist> `is that the unused variable detection can't see into child scopes` heh I just naswered a question about this on stackoverflow recently
20:17:25 <briantist> and then closed another one as a duplicate a day or 2 later
20:17:59 <briantist> https://stackoverflow.com/a/69847506/3905079
20:18:17 <nitzmahone> mkletz: any other discussion on next steps for the win_dsc stuff?
20:18:37 <mkletz> I'd like a sanity check on my general idea
20:19:16 <mkletz> so since a composite resource is many resources
20:19:34 <mkletz> I was thinking a bit of logic that basically determines how the objects to invoke are supplied
20:19:42 <mkletz> and have it end iterating through them at a single point
20:19:47 <mkletz> instead of two distinct workflows
20:19:50 <mkletz> if that makes sense?
20:20:34 <nitzmahone> +1 from me in one form or another; for my vote, how cleanly the impl can separate the composite/not cases and if there are any new deps required will determine whether it's immediately accepted as a change to win_dsc in ansible.windows or a new module in community.windows first. That sounds like a pretty sane way to do it.
20:20:58 <mkletz> I figure that way we can also slowly introduce the change over a few PRs
20:21:03 <jborean93> yea to me the devil is in the details and it's hard to comment without seeing the actual work
20:21:07 <nitzmahone> ditto
20:21:22 <mkletz> right, it's sort of a loose idea in my head at the moment. I'm still getting familiar with the win_dsc code
20:21:35 <jborean93> it's definitely one of the more gnarly ones out there
20:21:44 <jborean93> getting the module invocation wasn't fun
20:21:52 <mkletz> yeah, and we might actually have a customer use case for this now also
20:22:13 <mkletz> so more motivation to work on it
20:22:22 <briantist> I wouldn't be surprised if I end up using the implementation at work as well :)
20:22:25 <nitzmahone> Though it still sounds like the majority of the code is applicable to both cases, so it's really just "do we need to do the composite MOF compile step, and how many resource apply's are we doing, 1 or N?"
20:22:36 <mkletz> yeah basically
20:22:46 <mkletz> this is how the thing goes
20:22:50 <mkletz> does it go once or a lot
20:23:32 <nitzmahone> So if the impl can very cleanly separate those cases and keep the signal path clean for the existing stuff, I'd be +1 for just pulling it in to the supported module, but the code will have to do the talking ;)
20:23:43 <mkletz> yup for sure
20:24:07 <briantist> I think I agree with what Matt said
20:24:09 <nitzmahone> (but something as simple as adding a loop that might be 1 is much preferable to duplicating the code)
20:24:42 <jborean93> especially the win_dsc code
20:24:51 <mkletz> yeah, I just need to spend some time getting more familiar with that code needs to be in said loop
20:25:00 <mkletz> what code*
20:25:27 <mkletz> I'm confident it can work, just how pretty it is remains to be seen
20:25:40 <nitzmahone> We're waiting with bated breath ;)
20:25:44 <mkletz> lol
20:26:34 <nitzmahone> Anything else for today then?
20:26:42 <mkletz> not from me
20:27:22 <briantist> nor me
20:27:37 <nitzmahone> Cool- til next week then! Have a great Thanksgiving, for the US folks...
20:27:42 <jborean93> I'm all good
20:27:49 <nitzmahone> #endmeeting