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