20:01:39 <jborean93> #startmeeting Ansible Windows Working Group
20:01:39 <zodbot> Meeting started Tue Jan 15 20:01:39 2019 UTC.
20:01:39 <zodbot> This meeting is logged and archived in a public location.
20:01:39 <zodbot> The chair is jborean93. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:01:39 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
20:01:39 <zodbot> The meeting name has been set to 'ansible_windows_working_group'
20:01:42 <jborean93> hey
20:01:44 <nitzmahone> o/
20:01:51 <jborean93> #chair jhawkesworth_ nitzmahone
20:01:51 <zodbot> Current chairs: jborean93 jhawkesworth_ nitzmahone
20:01:55 <it-praktyk> hey
20:02:01 <jborean93> #chair it-praktyk
20:02:01 <zodbot> Current chairs: it-praktyk jborean93 jhawkesworth_ nitzmahone
20:02:45 <chopraaa> Hi
20:03:23 <jborean93> #chair chopraaa
20:03:23 <zodbot> Current chairs: chopraaa it-praktyk jborean93 jhawkesworth_ nitzmahone
20:03:31 <jhawkesworth_> wow big turn out today
20:04:13 <jborean93> yea, just checking the agenda right now
20:04:32 <jhawkesworth_> agenda: https://github.com/ansible/community/issues/420
20:05:02 <jborean93> looks like one from dag which is merely a request for a review on his timeout PRs
20:05:49 <chopraaa> Would like to bring about a discussion regarding code quality checks today (if possible)
20:06:18 <jborean93> The winrm ones are waiting on a change in pywinrm but I need to get to the psrp PR
20:06:33 <jborean93> #topic open floor
20:06:39 <jborean93> chopraaa: go ahead
20:07:11 <chopraaa> So, right now theres a huge parity in the kind of code that comes through in modules.
20:07:25 <chopraaa> Spacing, indentation, and in general - code smell
20:08:43 <chopraaa> Would like to follow general guidelines out forward by the powershell community if possible
20:09:04 <nitzmahone> Anything we can't automate the enforcement of is (IMO) a waste of time
20:09:07 <chopraaa> This would help non maintainers understand and update the code where possible
20:09:18 <jborean93> the limited code analysis is the rules we have set up for PSScriptAnalyzer but we decided not to enforce one standard around indentation (2 space v 4 space v tabs) and brace placements
20:09:43 <jborean93> but agreed with nitzmahone, unless we can test it there's not a good way to enforce it
20:10:11 <chopraaa> I had psscriptanalyzer in mind as well
20:10:30 <nitzmahone> It's already in use
20:10:37 <jborean93> if there are new rules, or rules we aren't running with we can discuss turning them on
20:10:39 <chopraaa> O.o
20:10:50 <jborean93> https://github.com/ansible/ansible/tree/devel/test/sanity/pslint
20:12:02 <chopraaa> Neat, thanks
20:12:09 <nitzmahone> Every pull request that contains PS code is run with those rules and not allowed to be merged until it passes
20:12:17 <nitzmahone> (by our CI system)
20:12:53 <jhawkesworth_> yeah if there are any more rules that could be applied, it would be worth considering.
20:13:31 <jhawkesworth_> IIRC ansible hasn't taken PRs that only reformat code due to the problems it causes with attributing code down the line.
20:13:40 <jborean93> you can also run it yourself by doing `ansible-test sanity --test pslint --docker default`
20:13:49 <jhawkesworth_> nice
20:13:52 <chopraaa> Theres https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/windows/win_psmodule.ps1 for example
20:14:13 <chopraaa> You can notice irregularities right off the bat
20:14:27 <nitzmahone> We're running with the default rules minus the ones in the settings file, so as PSSA is updated, it's (arguably incorrectly) running with any new rules in the default distribution
20:14:53 <chopraaa> Semicolons, different tab spacing, various different positions for the braces, and so on
20:15:45 <it-praktyk> the code of the update win_psmodule waits for the final (?) review
20:15:47 <nitzmahone> The hard part is that throwing pages of style at a contributor that can't be checked automatically makes for a bad contributor experience
20:16:02 <it-praktyk> so those smells were removed too
20:16:43 <chopraaa> Double edged sword :x
20:16:45 <jborean93> which ones in particular?
20:17:05 <nitzmahone> I think he means that the code is cleaned up in the recent PR
20:17:37 * chopraaa realises all swords are probably double edged.
20:17:40 <jborean93> ahh
20:18:42 <it-praktyk> BTW, I made a bad decision to close #46516 and open the new one PR #50621
20:19:25 <it-praktyk> I'll reopen the first one and I'll rebase that correctly
20:21:40 <jhawkesworth_> Personally I don't have a problem with improving the consistency of module code as part of a PR that makes a functional change - I'm sure there have been plenty like that...
20:22:41 <jhawkesworth_> .. but agree that unless checking is automatic it can become subjective and frustrating for contributors
20:23:33 <nitzmahone> and we usually discourage style-only changes
20:23:59 <nitzmahone> If you're in the neighborhood making functional changes, style changes are sometimes OK
20:27:43 <chopraaa> Yeah that makes sense
20:28:41 <jhawkesworth_> I guess we would be open to considering adding another thing that could check for consistent style.  A quick search hasn't turned anything up yet though.
20:29:06 <chopraaa> Possibly warnings instead of errors
20:29:06 <jhawkesworth_> well, I found this but it looks incomplete https://github.com/PoshCode/ScriptAnalyzer.PracticeAndStyle
20:29:53 <jhawkesworth_> not sure how I feel about warnings..  do you mean runtime warnings when you run ansible?
20:30:12 <jhawkesworth_> or warnings from CI before merging module code?
20:30:29 <jborean93> errors are better than warnings, people ignore warnings as long as they see a green light
20:30:35 <chopraaa> CI warnings
20:30:35 <jhawkesworth_> if CI then might as well make them errors
20:30:41 <chopraaa> Ah okay
20:32:31 <jhawkesworth_> if you have time to create something to check for style rules then make a PR.
20:33:04 <chopraaa> Will do
20:33:22 <jborean93> Is there anything else we would like to discuss?
20:34:09 <nitzmahone> nothing from me today
20:34:11 <jhawkesworth_> unless dag is around and wants to add anything to the connections prs?
20:34:46 <jhawkesworth_> but otherwise nothing else from me
20:34:55 <chopraaa> Nothing else from me
20:36:06 <jhawkesworth_> I'm going to review some PRs from this list here https://github.com/ansible/community/issues/420#issuecomment-450272566
20:36:21 <jhawkesworth_> but I can do that without an audience :-)
20:37:16 <jborean93> cool, if there's nothing else lets bring this to a close
20:38:39 <jborean93> closing in 3
20:38:42 <jborean93> 2
20:38:47 <jborean93> 1
20:38:50 <jborean93> #endmeeting