12:00:39 <gundalow> #startmeeting Ansible Community PR Review
12:00:39 <zodbot> Meeting started Thu Feb 21 12:00:39 2019 UTC.
12:00:39 <zodbot> This meeting is logged and archived in a public location.
12:00:39 <zodbot> The chair is gundalow. Information about MeetBot at http://wiki.debian.org/MeetBot.
12:00:39 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
12:00:39 <zodbot> The meeting name has been set to 'ansible_community_pr_review'
12:01:13 <gundalow> Hi everybody, who's around for the PR review day?
12:01:21 <kkao07> o/
12:01:23 * bmalynovytch raise hand
12:01:27 <LukasKaemmerling> o/
12:03:40 <gundalow> cabral404: Hi, you here for the PR review day?
12:04:07 <cabral404> @gundalow: yes!
12:05:49 <gundalow> Nice to see some new and familiar names joining us today. We try and keep this fairly informal. Please do shout out at any point with questions. Chances are if you have a question someone else does. Also feel free to message me directly `/query gundalow hi` if you'd like
12:08:18 <gundalow> We have a few aims of these PR review days
12:08:18 <gundalow> 1) Give people like yourself a place to learn how to contribute. Reviewing PRs is something that most people can help with, even if you don't know Python
12:08:18 <gundalow> 2) We will be looking at PRs not covered by Working Groups (https://github.com/ansible/community/wiki) these are PRs that often fall between the cracks and get forgotten about
12:08:18 <gundalow> 3) I hope that this will be fun and enjoyable, thank you all for taking the time to contribute
12:08:41 <gundalow> #chair kkao07 bmalynovytch[m] LukasKaemmerling cabral404
12:08:41 <zodbot> Current chairs: LukasKaemmerling bmalynovytch[m] cabral404 gundalow kkao07
12:08:54 <gundalow> #topic Bug PR review
12:09:16 <gundalow> #info First query https://github.com/ansible/ansible/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+-label%3Abackport++-label%3Asupport%3Acore+-label%3Asupport%3Acurated+-label%3Anetworking+-label%3Anew_module+-label%3Aaws+-label%3Aazure+-label%3Adocker+-label%3Alinode+-label%3Avmware++-label%3Awindows++label%3Abug+-label%3Aaws+-label%3Aazure+-label%3Adocker+-label%3Alinode+-label%3Anetworking+-label%3Avmware+-label%3Awindows
12:09:57 <gundalow> oh, woops, wrong URL
12:10:18 <gundalow> oh, no that's right, just different order
12:10:40 <gundalow> ^ All PRs that have `label:bug` and are not `support:core`, or covered by an active Working Group
12:11:06 <bmalynovytch[m]> I see MySQL PRs
12:11:40 <gundalow> bmalynovytch[m]: yes, I know the MySQL WG have been struggling a bit to get review, so I thought it maybe useful to include them in today's list
12:11:54 <gundalow> Maybe we can find some people that would like to join the MySQL WG :)
12:11:55 <bmalynovytch[m]> oh great !
12:12:09 <gundalow> bmalynovytch[m]: Good eyes, thanks for pointing it out
12:12:12 <bmalynovytch[m]> I feel a bit alone right now ^^
12:12:12 <gundalow> ok #1
12:12:21 <gundalow> https://github.com/ansible/ansible/pull/52699 os_coe_cluster module overrides param labels with wrong behavior #52699
12:12:41 * akasurde waves
12:12:47 <gundalow> #chair akasurde
12:12:47 <zodbot> Current chairs: LukasKaemmerling akasurde bmalynovytch[m] cabral404 gundalow kkao07
12:12:57 <gundalow> akasurde: Afternoon :)
12:13:05 <akasurde> gundalow GA
12:13:44 <gundalow> #52699 So we can see that this is from a `first-time contributor` to Ansible, which is great
12:14:38 <akasurde> yeah
12:14:58 <akasurde> gundalow There is failure in CI test
12:16:07 <akasurde> maybe we should move on to next in list
12:16:10 <gundalow> Yup, for OpenStack we have two sets of CI 1) Shippable 2) Openstack
12:16:11 <gundalow> yup
12:17:00 <gundalow> https://github.com/ansible/ansible/pull/52685 gcp_compute: use env variables on inventory script
12:17:59 <akasurde> I think this is in progress
12:18:12 <gundalow> yup, next
12:18:30 <gundalow> https://github.com/ansible/ansible/pull/52675 Fix redfish_facts GetPsuInventory command not returning correct output
12:18:56 <akasurde> No idea about Redfish
12:19:15 <bmalynovytch[m]> none either
12:20:07 <gundalow> baptistemm_: Hi, you here for the PR review day?
12:20:43 <gundalow> Looks like their are some maintainers for this
12:21:11 <gundalow> https://github.com/ansible/ansible/pull/52668 Use local dummy repo for flatpak_remote integration tests
12:21:47 <bmalynovytch[m]> O.O this one is fat
12:22:03 <gundalow> From the first file changed `test/integration/targets/flatpak_remote/aliases` we can see that the CI tests have gone from disabled (`unsupported`) to running
12:22:03 <akasurde> 80 Files
12:23:13 <akasurde> mattclay, might have more insights
12:23:18 <baptistemm_> I'm coming to look what happen here gundalow
12:23:20 <gundalow> I think the important bit is https://github.com/ansible/ansible/pull/52668/files#diff-14297304a23a7643ea7607126bc63290
12:23:34 <gundalow> #chair baptistemm_ alongchamps
12:23:34 <zodbot> Current chairs: LukasKaemmerling akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 gundalow kkao07
12:23:50 <gundalow> Welcome, we are currently going through bug fix PRs
12:24:05 <alongchamps> hi gundalow, working on some breakfast myself :)
12:24:21 <akasurde> alongchamps, hi
12:24:43 <gundalow> 52668: I see they have some questions
12:24:53 <alongchamps> hi akasurde!
12:25:16 <gundalow> #action ask mattclay to review 52668
12:25:47 <bmalynovytch[m]> gundalow: is it possible to see / know if those tests were run by shippable ?
12:25:59 <bmalynovytch[m]> this could help us review
12:26:16 <gundalow> bmalynovytch[m]: sure, click `show all checks` (in the green box), then `details`
12:26:25 <akasurde> bmalynovytch[m], yes you can click on "show details" next to show all chekcs
12:26:42 <bmalynovytch[m]> got it, though I can't find them specifically
12:27:00 <gundalow> from `aliases` file `shippable/posix/group3`
12:27:41 <bmalynovytch[m]> I'm not familiar with the shippable notations 😞
12:28:17 <gundalow> `group3` = `${OS}/3`
12:28:27 <gundalow> oh, tests haven't run
12:28:40 <gundalow> https://app.shippable.com/github/ansible/ansible/runs/108979/77/console
12:28:40 <gundalow> `WARNING: Excluding tests marked "needs/privileged" which require --docker-privileged to run under docker: flatpak_remote`
12:29:11 <akasurde> bmalynovytch[m], no these are ansible specific groups (manages the order of tests)
12:29:16 <bmalynovytch[m]> :/
12:29:47 <akasurde> #info https://docs.ansible.com/ansible/latest/dev_guide/testing_integration.html
12:29:58 <bmalynovytch[m]> akasurde: thx
12:30:03 <akasurde> np
12:30:41 <bmalynovytch[m]> (I also noticed the doc to rework documentation, as it's hard for contributers to find the right docs)
12:31:42 <akasurde> bmalynovytch[m], generally most of things are documented. For deveveloper https://docs.ansible.com/ansible/latest/dev_guide/
12:32:05 <akasurde> gundalow, next ?
12:32:14 <gundalow> bmalynovytch[m]: the docs, esp "how to test" need some work
12:32:33 <bmalynovytch[m]> that was my point the last few days :)
12:32:41 <gundalow> https://github.com/ansible/ansible/pull/52642 VMware: Refactor guest inventory plugin
12:32:51 <gundalow> akasurde: oh, missing vmware label.
12:32:52 <bmalynovytch[m]> as well as how to rebase stale PRs
12:33:28 <akasurde> it is under review, by bcoca and shertel but I'm happy if anyone wants to take a look
12:33:30 <gundalow> bmalynovytch[m]: rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html Ansibulbot will like to this page if their is a merge conflict
12:33:34 <bmalynovytch[m]> that one's for you akasurde ? ;)
12:33:52 <akasurde> yes
12:33:56 <gundalow> #action lib/ansible/plugins/inventory/vmware needs adding to BOTMETA
12:34:22 <akasurde> gundalow I am on it
12:34:24 <akasurde> :)
12:35:02 <gundalow> akasurde: not sure if we can merge Botmeta changes yet
12:35:03 <akasurde> gundalow sorry to break order but since we are on BOTMETA - https://github.com/ansible/ansible/pull/52704 can you take a look
12:36:06 <gundalow> akasurde: nps, approved, though need to check with Tanner if we can merge that yet
12:36:20 <gundalow> stroobl: Hi, you here for the PR review day/
12:36:20 <akasurde> gundalow sure
12:37:23 <stroobl> hi gundalow
12:37:36 <stroobl> well, I saw it passing by and joined :)
12:38:02 <gundalow> stroobl: Ace, welcome. We are looking at bugfix PRs today
12:38:04 <gundalow> #chair stroobl
12:38:04 <zodbot> Current chairs: LukasKaemmerling akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 gundalow kkao07 stroobl
12:38:17 <gundalow> https://github.com/ansible/ansible/pull/52594 redhat_subscription: Exempt register vars from shell expansion
12:38:37 <bmalynovytch[m]> comment suggest another PR which has already been merged
12:39:04 <cabral404> I think it was actually suggesting using a flag that was merged
12:39:17 <gundalow> hum, is that saying it's covered by 45620, or that options should be renamed, or use the funcationality in 45620
12:40:24 <bmalynovytch[m]> you're right, it's about the naming choice
12:40:45 <cabral404> cwilkers forced push, so I don't think we can see what he did in the first commit
12:40:54 <bmalynovytch[m]> in fact it seems to be relying on command module
12:41:10 <bmalynovytch[m]> so name MUST match the change made in command module
12:41:20 <akasurde> cabral404, https://github.com/ansible/ansible/commit/26f7c28346963ced5aaa2394f432bc9d16fe9e26
12:42:29 <cabral404> oh sorry, TIL. =)
12:42:49 <akasurde> cabral404, don't be sorry :)
12:42:54 <gundalow> akasurde: felixfontein FYI Ansibulbot issue has been resolved, so I think I've merged all the outstanding BOTMETA changes
12:43:02 <gundalow> aye, this is all about learning
12:45:30 <gundalow> 52594 Thoughts, is it good to merge?
12:45:49 <cabral404> looks good to me
12:46:22 <bmalynovytch[m]> seem good to me too
12:47:18 <webknjaz> lgtm
12:47:28 <gundalow> webknjaz: afternoon
12:47:59 <gundalow> Just looking if expand_user_and_vars madeit into 2.7
12:48:19 <webknjaz> ☕
12:48:30 <gundalow> expand_user_and_vars is in 2.8 only
12:49:06 <gundalow> #action merge 52594
12:49:24 <gundalow> #info We don't merge during today as this allows people to continue to review and add comments.
12:49:51 <gundalow> OK, slight tangent
12:50:03 <gundalow> https://github.com/ansible/ansible/pull/52553 Add Hetzner Cloud Inventory Plugin
12:50:08 <gundalow> from LukasKaemmerling
12:50:29 <gundalow> This is an Inventory Plugin
12:51:25 <LukasKaemmerling> yep, an inventory plugin for our Hetzner Cloud. We plan to contribute more things (modules) in the short future as well.
12:52:07 <LukasKaemmerling> FYI: a working group is also already requested
12:52:19 <gundalow> hum, I've not as familiar with inventory plugins, though I thought there is some built-in so you don't need to do `self.api_token = os.getenv("HCLOUD_TOKEN")`
12:52:23 * gundalow looks for an example
12:54:19 <gundalow> akasurde: webknjaz do you know if lookup plugin have a built in way to get ENV, rather than doing self.api_token = os.getenv("HCLOUD_TOKEN")
12:54:45 <webknjaz> no idea
12:54:54 <LukasKaemmerling> in as sample the linode inventory it is also `os.environ['LINODE_ACCESS_TOKEN']`
12:55:27 <akasurde_> gundalow need to check
12:55:44 <resmo> hi
12:55:53 <akasurde_> resmo, hi
12:56:07 <akasurde_> gundalow yes it does have ENV for taking variables
12:56:30 <gundalow> resmo: Hi, you joining PR review?
12:56:33 <felixfontein> gundalow: great, thanks! (sorry, mostly afk, have to do actual work ;) )
12:56:39 <akasurde_> gundalow https://github.com/ansible/ansible/blob/cdb25d0e69c71b6088cd62adac64e28d59d23e58/lib/ansible/plugins/lookup/etcd.py#L41
12:56:45 <gundalow> felixfontein: you are forgiven :P
12:57:28 * bmalynovytch[m] AFK for 1h, leading an interview
12:58:10 <gundalow> LukasKaemmerling: apart from this minor thing I think it's good
12:58:23 <gundalow> 52553: Anyone got any views?
12:59:08 <LukasKaemmerling> yep with this it works too. i will push the change directly.
12:59:31 <gundalow> webknjaz: Good comments, thanks
12:59:58 <webknjaz> LukasKaemmerling: I've added "nesting reduction" comments. this way it's easier to follow the code structure
13:00:02 <gundalow> +1
13:00:12 <LukasKaemmerling> okay, thank you! i will change it directly too :)
13:01:18 <gundalow> LukasKaemmerling: cool, ping me once you are happy and I'll take another look and merge
13:01:28 <gundalow> (via IRC, I get too much GitHub email :P)
13:01:40 <gundalow> ironfroggy: Hello, you here for the PR review day?
13:01:43 <cabral404> Python 2.7 is going to reach EOL at the end of the year. Are you guys still maintaining until then?
13:02:08 <cabral404> Python 2.7 is going to reach EOL at the end of the year. Are you guys still maintaining support until then?
13:02:11 <LukasKaemmerling> gundalow: is pushed, let's wait on the ci but i guess there shouldn't be something wrong
13:02:57 <webknjaz> cabral404: probably even further. but we'll have think about dropping py2 support on controller side at some point
13:03:21 <cabral404> alright
13:03:31 <akasurde_> LukasKaemmerling, just out-of-curiosity are you not supporting cache for hcloud plugin
13:04:29 <LukasKaemmerling> yep for the moment this is correct. We first want to see how much traffic comes from this :) If it is needed, we plan to integrate the cache too.
13:04:45 <gundalow> cabral404: I think it's written down somewhere, though Ansible will be supporting Python 2.7 on the remote-node for sometime as there are RHEL OS that still support Py2.7, and RHEL has a long support tail
13:05:03 <gundalow> LukasKaemmerling: +1 to start simple, get feedback and build on it
13:05:11 <cabral404> got it. makes sense.
13:05:19 <akasurde_> OK
13:05:24 <akasurde_> LukasKaemmerling, ok
13:05:52 <gundalow> LukasKaemmerling: Any other feedback we can give you at this time?
13:06:41 <resmo> gundalow, not really no, probably later
13:06:43 <LukasKaemmerling> nope, thanks! I think when the ci is green everything should be okay and ready.
13:07:03 <webknjaz> LukasKaemmerling: here's how you can save on iterations and list recreation: https://github.com/ansible/ansible/pull/52553/files#diff-4f000a93ee8c5d06d5a245c8e169b6eeR133
13:07:40 <gundalow> #action See if plugins/inventory have some built-in for reading env, if so update https://docs.ansible.com/ansible/devel/plugins/inventory.html
13:08:03 <gundalow> https://github.com/ansible/ansible/pull/52574 postgresql_privs change fail to warn if role(s) does not exist
13:08:08 * resmo is looking at 52553
13:09:44 <resmo> LukasKaemmerling, hi, I already looks once at that PR and one thing I noted is that htere is a part in the code in which you initialize a tmp var serveral times, so the previous valaues get lost
13:10:11 <resmo> e.g https://github.com/ansible/ansible/pull/52553/files#diff-4f000a93ee8c5d06d5a245c8e169b6eeR138
13:10:33 <LukasKaemmerling> resmo: nope they didn't get lost because i overwrite the base value everytime (self.servers = tmp)
13:11:15 <webknjaz> resmo: I've posted a refactor suggestion a few lines upper
13:12:20 <resmo> LukasKaemmerling, this looks very "golang-ish" len(self.get_option("images")) > 0: :)
13:14:14 <resmo> any reason why not just "if self.get_option("images"):" ?
13:14:31 <resmo> not a blocker but anyway
13:15:10 <webknjaz> I think it's not needed at all and my patch optimizes it to be just one iteration pass...
13:15:10 <themroc> catching up ...
13:16:23 <LukasKaemmerling> webknjaz: tested your suggestion but it doesn't work, (tested with location: - fsn1) I will try to apply your optimization.
13:16:42 <LukasKaemmerling> resmo: i think this is because im coming from go :) i will change it!
13:16:42 <webknjaz> hm..
13:16:53 <webknjaz> welcome to coding in browser comment box :)
13:17:21 <felixfontein> almost as much fun as coding on paper
13:18:21 <webknjaz> LukasKaemmerling: after applying my suggestion you also need to remove previous lines. Unfortunately github only allows attaching those to one line :(
13:19:32 <webknjaz> oh, my logic is off.
13:19:58 <webknjaz> you want to find servers matching all of the checks, right?
13:20:05 <LukasKaemmerling> yep :)
13:20:35 <webknjaz> I've changed the snippet in comment
13:20:43 <webknjaz> should be better now
13:24:27 <resmo> LukasKaemmerling, out of curiosity I noticed you have ansible modules in go for hcloud, any plans to translate them to python?
13:24:35 <LukasKaemmerling> yep :)
13:24:41 <LukasKaemmerling> this will be the next steps
13:24:59 <LukasKaemmerling> (we plan to contribute them directly to ansible)
13:25:07 <resmo> (I actually contacted the support to get an account for development of these modules, few weeks later I saw you created them in go...)
13:25:35 <resmo> LukasKaemmerling, perfect!
13:26:07 <akasurde_> gundalow want to move to next ?
13:26:07 <gundalow> LukasKaemmerling: looks like resmo just volunteered to join your new Hetzner WG :)
13:26:42 <resmo> LukasKaemmerling, a.o. I authored the cloudstack and vulr integration in ansible, working on cloudscale currently, upcloud is on my list as well
13:26:53 <resmo> s/vulr/vultr
13:26:55 <gundalow> https://github.com/ansible/ansible/pull/52564 pids: case insensitive string comparison for process names
13:26:59 <gundalow> One line change
13:27:07 <akasurde_> LGTM
13:27:31 <LukasKaemmerling> resmo: i will write you (properly tomorrow). Sorry guys i must leave the meeting now :)
13:27:47 <gundalow> LukasKaemmerling: Thanks for joining :)
13:27:52 <akasurde_> LukasKaemmerling, include me as well :)
13:28:00 <LukasKaemmerling> okay :)
13:28:58 * resmo bbl
13:29:02 <ironfroggy> gundalow: i didn't know anything about it. should i be?
13:29:33 <gundalow> ironfroggy: Only if you'd like to be https://github.com/ansible/community/issues/407 for background
13:30:00 <gundalow> #action merge 52564 (no backport, new in 2.8)
13:30:03 <gundalow> akasurde_: Thanks
13:31:01 <gundalow> https://github.com/ansible/ansible/pull/52452 osx_defaults: refactor
13:31:06 <gundalow> Any OSX users here?
13:31:11 <ironfroggy> i will sit in and see what its all about
13:31:12 <akasurde_> yeah !!!
13:31:15 <alongchamps> yep
13:32:41 <gundalow> hvtuananh: Hi, you here for the PR review day?
13:32:52 <hvtuananh> hi yes
13:32:53 <gundalow> ironfroggy: ace, Please shout out any anypoing :)
13:32:59 <gundalow> #chair hvtuananh
13:32:59 <zodbot> Current chairs: LukasKaemmerling akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 gundalow hvtuananh kkao07 stroobl
13:33:14 <gundalow> hvtuananh: Excellent, welcome. We are currently looking at bugfix PRs
13:33:25 <hvtuananh> I have a bugfix PR open for more than a year and would love some feedback
13:33:26 <gundalow> hvtuananh: currently https://github.com/ansible/ansible/pull/52452
13:33:27 <themroc> #chair themroc
13:33:33 <gundalow> #chair themroc
13:33:33 <zodbot> Current chairs: LukasKaemmerling akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 gundalow hvtuananh kkao07 stroobl themroc
13:33:46 <gundalow> hvtuananh: sure, got a link, will do that one next
13:33:59 <akasurde> #chair ironfroggy
13:33:59 <zodbot> Current chairs: LukasKaemmerling akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 gundalow hvtuananh ironfroggy kkao07 stroobl themroc
13:34:27 <hvtuananh> sure mine is https://github.com/ansible/ansible/pull/36811
13:35:46 <gundalow> 52452  akasurde Looks like Dag's comments have been addressed
13:36:14 <akasurde> Yes
13:37:12 <gundalow> LGTM
13:37:23 * gundalow spots a dag
13:37:26 <dag> 52452 merged :-)
13:37:34 <gundalow> dag: Thanks!
13:37:40 <dag> akasurde: thanks for going the extra mile !
13:38:01 <gundalow> https://github.com/ansible/ansible/pull/52234 Fix reviews issues for scaleway_lb
13:38:04 <dag> akasurde: would be hard to get people testing this at this point in time, we don't have an active WG yet
13:38:09 <akasurde> Thanks dag, I appreciate your reviews and comments
13:38:46 <gundalow> 52234 is updating a module which is new in 2.8, so it's OK that the modules options have been renamed
13:39:11 <dag> akasurde: If you like to join the macOS WG, you're welcome *hint* ;-)
13:39:34 <akasurde> dag, definitely I am in
13:39:38 <dag> Scaleway ? Is the vendor not involved ?
13:40:25 <akasurde> #chair dag
13:40:25 <zodbot> Current chairs: LukasKaemmerling akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 dag gundalow hvtuananh ironfroggy kkao07 stroobl themroc
13:40:57 <gundalow> dag: hum, not sure. I've seen the name mentioned a few times
13:41:00 <dag> gundalow: Not sure if I can do a lot today, but ping me if something has my name on it :)
13:41:05 <themroc> dag, commit is from initial author of the module
13:41:15 <dag> ok, in that case it's fine
13:42:11 <gundalow> I've pinged Pilou, though I know they've been busy with $day_job recently
13:42:38 <akasurde> brb
13:43:06 <gundalow> #action merge 52234 (no backport)
13:43:28 <gundalow> https://github.com/ansible/ansible/pull/52166 Fix idempotence in rabbitmq_plugin
13:43:29 <themroc> the comment fro Pilou is here:  https://github.com/ansible/ansible/pull/51741
13:45:19 * dag noticed the vmware WG has 2 new members added
13:46:55 <gundalow> themroc: ah, thanks
13:46:59 <gundalow> dag: win
13:48:49 <gundalow> #action merge & backport 52166
13:50:06 <gundalow> https://github.com/ansible/ansible/pull/52721 identity: Issue warning if GSSAPI parameters can't be used
13:51:03 <Pilou> about #52234, ansible keywords should be used not module parameters, isn't it ?
13:52:00 <gundalow> hey Pilou :)
13:52:02 <gundalow> hum, not sure
13:52:28 <gundalow> dag: ^ from Pilou
13:54:21 <gundalow> #action 52721 merge (no backport)
13:55:28 <dag> gundalow: which one ?
13:55:28 <gundalow> https://github.com/ansible/ansible/pull/52117 archive: Fix empty files to dest
13:55:39 <gundalow> dag: about #52234, ansible keywords should be used not module parameters, isn't it ?
13:56:14 <bmalynovytch[m]> gundalow: I'm back
13:56:39 <gundalow> bmalynovytch[m]: \o/
13:56:53 <bmalynovytch[m]> lol sorry for the delay
13:57:01 <dag> gundalow: I think he is modifying the interface, but that's plain wrong
13:57:19 <gundalow> 52117: I'd want to see tests
13:59:15 <dag> https://github.com/ansible/ansible/pull/52234#pullrequestreview-206316858
14:00:06 <gundalow> dag: thanks
14:00:16 <gundalow> https://github.com/ansible/ansible/pull/52117 archive: Fix empty files to dest
14:00:55 <gundalow> Not sure if that's a good fix or not
14:01:29 <bmalynovytch[m]> 52117: I agree with Dag's review
14:01:43 <bmalynovytch[m]> sorry, ment 52234
14:02:05 <gundalow> bmalynovytch[m]: feel free to add that to the PR
14:02:59 <gundalow> bmalynovytch[m]: 👍 thanks
14:05:55 <gundalow> https://github.com/ansible/ansible/pull/52004 terraform: patch state 'planned' outputs and perform minor refactor
14:06:07 <gundalow> dbpiv: rharolde Hi, you here for the PR review day?
14:07:27 <dbpiv> @gundalow Not intentionally.  IRC was just auto connecting.  But if it is now, I will probably stick around and see what is happening.
14:08:08 <gundalow> dbpiv: ah, OK https://github.com/ansible/community/issues/407 for context, feel free to idle. Please do shout out if you see something interesting :)
14:09:12 <bmalynovytch[m]> 52004: parameters were renamed, not directly concerning the issue it tries to cover. Are the renames necessary ?
14:09:33 <dbpiv> @gundalow I was aware of context....  Just not sure if I could contribute.  Fly on the wall.  But since I am here.  I am going to patch to the latest to see if the only bug I care about has been merged in.
14:11:04 <gundalow> https://github.com/ansible/ansible/pull/36811 Fixed lvol ValueError with float size.
14:11:11 <gundalow> hvtuananh: appologies, I missed this earlier
14:14:14 <hvtuananh> no worry :)
14:15:40 <gundalow> hvtuananh: `locale.setlocale(locale.LC_ALL, '')` I *thought* Ansible did that automatically before running any code
14:17:34 <gundalow> oh, nm
14:18:26 <felixfontein> /2/2
14:18:35 <felixfontein> meh
14:18:51 <gundalow> hvtuananh: given https://github.com/ansible/ansible/issues/32886#issuecomment-424009558 I think we are good
14:19:32 <gundalow> hvtuananh: could you please add a bugfix  changelog/fragment https://docs.ansible.com/ansible/devel/community/development_process.html#making-your-pr-merge-worthy
14:20:47 <hvtuananh> absolutely. I'm leaving for work now but will be online in the next hour to make the change.
14:21:21 <gundalow> #action review and possibly merge 36811
14:21:24 <gundalow> hvtuananh: Thanks!
14:23:15 <gundalow> #action merge 52004
14:23:37 <bmalynovytch[m]> gundalow: regarding the MySQL PRs, I suggest that we first review my own PR (#45355) first, as I reworked in depth the mysql_user module which includes many bugfixes, and will require other PRs to be rebased on it
14:23:39 <gundalow> https://github.com/ansible/ansible/pull/51953 onepassword_facts bug fixes
14:23:59 <dag> gundalow: cyberpear requested OpenStack WG status, and I created the Wiki and Pinboard: https://github.com/ansible/community/wiki/OpenStack
14:24:02 <gundalow> bmalynovytch[m]: ah, yes I skipped over a few MySQL PRs that makes sense
14:24:07 <gundalow> dag++
14:24:07 <zodbot> gundalow: Karma for dag changed to 1 (for the current release cycle):  https://badges.fedoraproject.org/tags/cookie/any
14:24:40 <dag> gundalow: we got 6 extra members even when we already had 8 :-)
14:24:59 <dag> gundalow: would be nice if we could attract a few good leads in this one
14:25:10 <dag> still one month before the freeze...
14:25:33 <gundalow> dag: yes. Can you remind me tomorrow?
14:25:40 <dag> sure
14:25:47 <dag> it's in the spreadsheet
14:26:06 <gundalow> 51953: So newer versions of onepass change the format, so I'm wondering if this PR will break for older versions of onepass
14:26:16 <gundalow> I'm assuming onepass is something that you have installed locally
14:29:16 <gundalow> 51953 I've left a comment
14:30:21 <gundalow> https://github.com/ansible/ansible/pull/45355 mysql_user: fix compatibility issues with various MySQL/MariaDB versions
14:30:25 <bmalynovytch[m]> gundalow: there's a problem with formating of your comment on 51953
14:30:26 <gundalow> from bmalynovytch[m]
14:30:34 <bmalynovytch[m]> o/
14:30:55 <gundalow> bmalynovytch[m]: ah, thanks, always forget you need a blank line after `> quote`
14:31:15 <gundalow> Anyone here use MySQL?
14:31:44 <gundalow> bmalynovytch[m]: you OK to lead the discussion, I need to setup out for 10 minutes
14:32:09 <bmalynovytch[m]> fine for me
14:32:33 <bmalynovytch[m]> so, the main issue sticks there since 2.2
14:32:54 <bmalynovytch[m]> MariaDB 10.2+ and MySQL 5.7+ introduced new privilege formats
14:33:15 <bmalynovytch[m]> the module in its current form doesn't handle the properly
14:33:44 <bmalynovytch[m]> this rewrite aims to setup the main code structure to handle old and new versions smoothly
14:35:11 <bmalynovytch[m]> what the module in the PR doesn't do is handling all new authentication sources available as of now (its goal for now is to ensure compatibility with native user management)
14:35:23 <mrproper> Is there documentation about what we're attacking and how today?
14:36:05 <bmalynovytch[m]> and another contributor noticed an edge case where idempotence wasn't full (password identified as always changed, when it isn't)
14:36:34 <bmalynovytch[m]> this small regression should be fixed in another PR which is awaiting this one to be merge
14:36:35 <bmalynovytch[m]> merged
14:37:08 <bmalynovytch[m]> mrproper: https://github.com/ansible/ansible/pull/45355#issuecomment-428200244
14:40:21 <bmalynovytch[m]> gundalow: discussion seem to be a monolog lol
14:45:08 <gundalow> hehe
14:45:58 <gundalow> mrproper: Hi, we are looking at bugfix PRs that are not covered by the Core Team, or a Working Group (https://github.com/ansible/community/wiki), ie the stuff that would otherwise fall between the cracks
14:46:15 <gundalow> mrproper: https://github.com/ansible/community/issues/407 for context
14:46:42 <bmalynovytch[m]> mrproper: sorry, I misread your question 🤦
14:46:54 <winem_> hey guys, sorry for being late :)
14:49:13 <gundalow> mrproper: please shout out if you have questions
14:49:19 <gundalow> #chair mrproper winem_
14:49:19 <zodbot> Current chairs: LukasKaemmerling akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 dag gundalow hvtuananh ironfroggy kkao07 mrproper stroobl themroc winem_
14:49:27 <gundalow> winem_: Welcome :)
14:50:04 <gundalow> bmalynovytch[m]: I think https://github.com/ansible/ansible/pull/45355 is close, just need to work out if there are any outstanding `-1`'s left
14:50:49 <gundalow> https://github.com/ansible/ansible/pull/51938 rhsm_repository: Properly handle no repos
14:52:26 <gundalow> #action merge 51938  & backport once changelog has been added
14:52:27 <bmalynovytch[m]> gundalow: thx !
14:52:58 <gundalow> https://github.com/ansible/ansible/pull/51916 extra_args_precommand parameter added to zypper_repository module
14:54:29 <gundalow> dag: all your comments on https://github.com/ansible/ansible/pull/51916 have been marked as resolved
14:55:46 <acozine> o/
14:56:15 <acozine> good morning (or appropriate greeting for your TZ)
14:56:24 <gundalow> #chair acozine
14:56:24 <zodbot> Current chairs: LukasKaemmerling acozine akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 dag gundalow hvtuananh ironfroggy kkao07 mrproper stroobl themroc winem_
14:56:26 <gundalow> acozine: Morning :)
14:57:16 <akasurde> acozine, morning
14:57:20 <gundalow> #action merge 51916
14:57:33 <acozine> akasurde: evening!
14:59:23 <acozine> how are things going? do we have a tally for the day so far?
15:00:47 <gundalow> acozine: I think we've got about dozzen to merge, and on the 2nd page of PRs
15:01:07 <acozine> Progress!
15:01:29 <gundalow> https://github.com/ansible/ansible/pull/51721 flatpak_remote: Fixing out of index error
15:02:05 <bmalynovytch[m]> hmmm
15:02:25 <bmalynovytch[m]> There will probably be some regressions with this
15:02:47 <gundalow> oh?
15:03:42 <bmalynovytch[m]> I read: "Looks like the flatpak command output changed from using spaces to tabs"
15:04:11 <bmalynovytch[m]> I understand that switching barely from space to tab will break old versions
15:05:24 <gundalow> ah, yes
15:06:58 <bmalynovytch[m]> would be better to split either on space OR tab
15:07:38 <bmalynovytch[m]> I add a review + comment
15:07:41 <gundalow> Thaks
15:08:07 <bmalynovytch[m]> oh you already did 3 days ago
15:08:12 <bmalynovytch[m]> but didn't ask for changes
15:08:18 <bmalynovytch[m]> https://github.com/ansible/ansible/pull/51721#issuecomment-464827401
15:08:41 <gundalow> ah, yes, as I wasn't sure if it would break or not
15:08:56 <bmalynovytch[m]> I'm not either
15:09:12 <bmalynovytch[m]> but it sounds pretty sure ^^
15:10:55 <navalkp> Hi @gundalow joining late, looks like Big PR review is over
15:11:23 <gundalow> navalkp: nop, still running
15:11:32 <gundalow> #chair navalkp
15:11:32 <zodbot> Current chairs: LukasKaemmerling acozine akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 dag gundalow hvtuananh ironfroggy kkao07 mrproper navalkp stroobl themroc winem_
15:11:49 <navalkp> Good
15:11:54 <gundalow> https://github.com/ansible/ansible/pull/51594 crypttab: Trim trailing newlines
15:11:56 <gundalow> navalkp: Welcome :)
15:12:45 <bmalynovytch[m]> gundalow: #51721 asked to ensure compat
15:12:59 <gundalow> bmalynovytch[m]: Thanks
15:14:08 <gundalow> https://github.com/ansible/ansible/pull/51594 crypttab: Trim trailing newlines
15:14:20 <gundalow> #51594 Looks sensible enough
15:14:39 <bmalynovytch[m]> gundalow: reviewed and approved #51594
15:15:17 <gundalow> #action merge & backport (with changelog) 51594
15:15:42 <bmalynovytch[m]> in fact, the change could also be handled further
15:15:55 <bmalynovytch[m]> https://github.com/ansible/ansible/blob/d7f334b92164775b51e386dd3fe62e1466e0184a/lib/ansible/modules/system/crypttab.py#L245
15:16:03 <gundalow> https://github.com/ansible/ansible/pull/51541 lxd_container fix to check for snap package install unix.socket
15:16:08 <bmalynovytch[m]> the line is then split before being reassembled
15:17:42 <bmalynovytch[m]> dunno
15:20:24 <gundalow> 51541 I've pinged the reporters of the original issue
15:21:13 <gundalow> #action see if 51541 has +1's and merge if needed
15:24:52 <gundalow> https://github.com/ansible/ansible/pull/51505 https://github.com/ansible/ansible/pull/51505
15:24:56 <gundalow> expect: utf-8' codec can't decode byte 0x80 enhancement #46556
15:24:58 <gundalow> Added some comments
15:25:04 <gundalow> hum, also feature, not bug
15:27:00 <gundalow> https://github.com/ansible/ansible/pull/51130 Update filetree.py to allow lists as input
15:29:01 <bmalynovytch[m]> gundalow: it seems to be a feature, not a bug
15:29:08 <gundalow> aye
15:30:02 <bmalynovytch[m]> the PR doesn't include a test example and code needs indepth review
15:30:50 <gundalow> Hum, lack of `@mention` from the Bot
15:31:11 <gundalow> #action gundalow review BOTMETA and add (lookup) plugin maintainers
15:31:39 <sivel> I'm not sure what 51130 is trying to solve, but I'm guessing it's not actually needed
15:32:08 <gundalow> sivel: aye, I wasn't sure
15:32:15 <gundalow> Also, Morning :)
15:32:33 <bmalynovytch[m]> I guess it's trying to get a list of files, based on multiple entries
15:33:34 <bmalynovytch[m]> > when_filtree:
15:33:45 <bmalynovytch[m]> shloudn't it be `with_filetree` ?
15:35:19 <gundalow> bmalynovytch[m]: oh, yes
15:36:13 * gundalow updates to `with_`
15:36:43 <gundalow> https://github.com/ansible/ansible/pull/51064 fix(rabbitmq_plugin): fixed an issue, when module successfully reports installation of non existing plugins
15:38:29 <gundalow> hum, never used, though looks OK
15:41:21 <gundalow> #action merge 51064
15:45:22 <gundalow> https://github.com/ansible/ansible/pull/50763 Fix Grafana dashboard overwriting
15:45:24 <gundalow> Anyone use Grafana?
15:45:53 <bmalynovytch[m]> I do but not with Ansible
15:46:08 <bmalynovytch[m]> giving it a quick eye, then got to go
15:46:30 <gundalow> bmalynovytch[m]: Ace, thank you for your time today :)
15:46:44 <bmalynovytch[m]> you're welcome :)
15:46:50 <acozine> what about https://github.com/ansible/ansible/pull/24550? Looks like `rebuild_merge` failed for some reason?
15:47:02 <bmalynovytch[m]> can't help "quickly" on #50763
15:47:02 <acozine> or were there other blockers?
15:47:07 <orthanc> I'm slow, but on #51064, might be better to apply the "does this plugin exist" check regardless of whether enabling or disabling
15:47:42 <gundalow> #action 24550 check CI status and merge once stale_ci has been removed
15:48:30 <gundalow> orthanc: Not slow at all, these meetings often end up with various topics going on at once.
15:48:43 <bmalynovytch[m]> leaving for today, bye guys !
15:48:44 <gundalow> orthanc: that's good feedback, would you be able to add that to the PR
15:48:48 <gundalow> bmalynovytch[m]: Thanks again
15:49:04 <bmalynovytch[m]> :)
15:49:04 <orthanc> gundalow will do
15:49:34 <acozine> I'll toss https://github.com/ansible/ansible/pull/33740 into the ring as well
15:51:49 <gundalow> #chair acozine
15:51:49 <zodbot> Current chairs: LukasKaemmerling acozine akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 dag gundalow hvtuananh ironfroggy kkao07 mrproper navalkp stroobl themroc winem_
15:51:53 <gundalow> How's still around?
15:52:00 <gundalow> Wondering if we should swap to Docs PRs?
15:52:14 <acozine> heh, I'd sneakily started in already ;)
15:52:20 * gundalow noted
15:53:36 <samccann> o/
15:53:53 <winem_> o/
15:54:26 <alongchamps> o/
15:55:27 <bcoca> -1 to not keeping an alias to the old name
15:55:35 <bcoca> just add symlink
15:56:21 <acozine> bcoca: is adding an alias an easy fix we could do as a suggestion?
15:56:29 <bcoca> posted on pr
15:56:33 <acozine> thx
15:56:38 <bcoca> ln -s newname oldname
15:59:57 <gundalow> someone else OK to drive for ~20 minutes or so
16:00:13 <gundalow> Just been doing URL #1234 ${title}
16:01:51 <gundalow> oh https://github.com/ansible/ansible/pulls?page=2&q=is%3Apr+is%3Aopen+-label%3Abackport++-label%3Asupport%3Acore+-label%3Asupport%3Acurated+-label%3Anetworking+-label%3Anew_module+-label%3Aaws+-label%3Aazure+-label%3Adocker+-label%3Alinode+-label%3Avmware++-label%3Awindows++label%3Abug+-label%3Aaws+-label%3Aazure+-label%3Adocker+-label%3Alinode+-label%3Anetworking+-label%3Avmware+-label%3Awindows&utf8=%E2%9C%93 just done 50763
16:02:29 <themroc> got to go, bye
16:02:30 <gundalow> themroc: Thanks for joining
16:02:54 <themroc> i was nort really helpfull, but i learned a bit
16:08:29 <hvtuananh> gundalow: I'm back, and added changelog fragment to my PR: https://github.com/ansible/ansible/pull/36811
16:08:51 <samccann> hvtuananh: thanks!
16:09:10 <samccann> anyone know which PR we were up to on the list gundalow put out ^^  I came in late
16:10:33 <samccann> ok gonna make a stab at it - let's look at https://github.com/ansible/ansible/pull/50586
16:11:12 <bcoca> has comment taht should be addressed, cfgparser from six
16:12:17 <samccann> ok thanks... let's try https://github.com/ansible/ansible/pull/50415
16:12:48 <bcoca> only ipa i care about has to do with beer ...
16:13:08 <samccann> heh
16:15:20 <samccann> would it be as simple as creating a couple of VMs with different ubuntu versions to test this out? or not worth the effort? not sure of the relative priority here
16:15:50 <orthanc> Regarding #50763, Not a graphana user but I can follow what the code is doing and it's clearly fixing the stated bug, have put a comment on the PR
16:17:40 <samccann> Thanks orthanc !
16:18:20 <mrproper> orthanc: I have Grafana at home so if you need something tested and there's a playbook and whatnot, I can test at home tonight possibly.
16:19:00 <dag> IPA and Grafana are on the list to become WGs
16:19:12 <dag> if there are any takers ;-)
16:19:21 <winem_> same here. grafana is available if any tests are required :)
16:19:36 * dag sees 2 potential members !
16:19:43 * mrproper lowers his hands
16:19:54 <mrproper> I barely use it so I'm hardly a good person to be on a WG.
16:20:18 <bcoca> i normally setup graphana for other people to use (and stop asking me for reports)
16:20:29 <samccann> thanks mrproper and winem_  If I look at #50763 it looks like the PR description includes a way to test it, so would appriciate it if you both can look and then comment in the PR after you try a test.
16:20:33 <dag> mrproper: you have 20+ modules on your name, you are _the_ person to review contributions :-D
16:21:10 <mrproper> git rm -rf meraki/ && git commit -m "Zero modules to my name" && git push
16:22:06 <orthanc> Personally I'd be comfortable trusting the submitter has checked it fixes their bug, it doesn't look like it would be a high risk of breaking other usages from what I can tell unless the graphana API does something really weird and unusual
16:22:44 <orthanc> Is the expectation that PRs like that are tested independantly or just reviewed?
16:23:19 <bcoca> tested and have tests
16:23:36 <bcoca> if you do pure code review, you should reflect that in your approval
16:23:47 <bcoca> i.e 'code lgtm but have not tested'
16:25:10 <orthanc> thanks boca have clarified the comment on PR to reflect that
16:25:32 <winem_> samccann: will test it and add a comment once I'm done
16:25:41 <samccann> cool thanks to you both!
16:26:11 <samccann> let's try a look at https://github.com/ansible/ansible/pull/50285
16:27:46 <moshloop_> hi
16:27:53 <samccann> anyone with openstack experience? ^^
16:28:04 <samccann> moshloop_ welcome!
16:30:52 <moshloop_> I have a few PR's that are waiting for merge, and couple more modules that I can create PR's for
16:31:52 <samccann> bcoca: https://github.com/ansible/ansible/pull/50379 - been over a month since your comment. Is it worth another nudge to the PR owner to see if they plan on coming back to that pr to address your comment? (not sure what's considered good practice here)
16:32:32 <bcoca> comment is style mostly, not showstopper
16:33:07 <bcoca> actually 2nd comment is
16:33:47 <bcoca> idk what we do for community now when author is unresponsive
16:33:49 <samccann> moshloop_ looks like your PRs are vmware related?  Folks - is there a vmware WG or did I just dream that up
16:34:29 <samccann> bcoca: for now I'll put a comment/nudge to see if it gets attention. gundalow can let us know what else might be worth trying when he gets back
16:36:21 <samccann> moshloop_: Okay I didn't dream it up - there is a vmware working group with regular meetings - https://github.com/ansible/community/wiki/VMware
16:36:25 <alongchamps> there is a vmware WG and channel
16:36:39 <samccann> thanks alongchamps !
16:36:42 <alongchamps> :)
16:37:25 <gundalow> back
16:37:39 <samccann> moshloop_ looks like you can add your prs to the vmware WG agenda here - https://github.com/ansible/community/labels/vmware  that might shake loose some reviews/merges etc
16:38:00 <alongchamps> next VMware WG meeting on Monday
16:38:10 * samccann hands steering wheel back to gundalow before she drives off a cliff
16:40:10 <gundalow> :D
16:40:47 <moshloop_> Ok - there are also a couple I have in the backlog that I am not sure if they are worth creating PR's for: 1) a module to generate a cloud-init ISO image  2) a module to generate and install a systemd service  3) a debug filter to print objects to stdout from inside jinja tempates
16:42:04 <gundalow> #action merge 49981
16:42:22 * alongchamps taking a break for lunch
16:42:23 <gundalow> samccann: was there anything that's good to merge, or I need to action?
16:42:55 <samccann> gundalow: no... we did get some comments added to PRs and someone or two folks  going to test the graphana PR
16:43:01 <gundalow> Ace, thanks
16:43:16 <gundalow> https://github.com/ansible/ansible/pull/49943 Fix netbox url with string concat
16:44:42 <gundalow> #action review, merge, add comment about backport on 49776
16:45:55 <gundalow> OK, ,ooks likes we've caught up with last big PR review 19th Dec 2018
16:46:08 <gundalow> acozine: samccann Shall we do docs next?
16:46:28 <acozine> sure, we've tossed a few in already
16:46:59 <gundalow> #info we've covered 52721 - 49665, which takes us to last PR review (19th Dec 2018)
16:47:05 <gundalow> #topic Docs PR review
16:47:39 <winem_> successfully tested a few test cases reg #50763
16:47:55 <samccann> thanks winem_ !!
16:47:56 <acozine> I'm going to grab the branch for 33740 and see if I can quickly address the comments on it, rebase it, and present it again for merge
16:48:42 <gundalow> \o/
16:48:48 <gundalow> #info Docs PRs https://github.com/ansible/ansible/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+label%3Adocs+-label%3Awip+sort%3Acreated-asc+
16:49:00 <moshloop_> I also have 2 non-vmware PR's https://github.com/ansible/ansible/pull/52731 and  and https://github.com/ansible/ansible/pull/45397
16:49:42 <gundalow> moshloop_: Can you please add the issue template?
16:50:11 <samccann> it looks like the rebuild_merge isn't working on this pr ? - https://github.com/ansible/ansible/pull/24550 ... any other tricks to try?
16:50:26 <gundalow> #action merge 24550
16:50:30 <gundalow> samccann: I'll look tomorrow
16:50:39 <gundalow> feel free to just `#action merge #1234`
16:50:59 <gundalow> OK, so onto the docs PRs
16:51:21 <gundalow> https://github.com/ansible/ansible/pull/13620 update_json for module intermediate comm
16:51:24 <gundalow> from bcoca
16:52:16 <winem_> time to get home, will be back in 1-2 hours if you're not already done by then :)
16:52:44 <samccann> thanks winem_ !!
16:52:49 <gundalow> winem_: Will see, thanks for your time!
16:53:03 <bcoca> gundalow: skip that one
16:53:22 <gundalow> OK
16:53:27 <acozine> bcoca: is that one WIP?
16:53:41 <gundalow> new lookup plugin: LMDB #24550
16:53:42 <bcoca> not really, more like Worked delayed due to external deps
16:53:50 <acozine> heh
16:53:51 <gundalow> #action merge 24550
16:54:12 <moshloop_> gundalow: Like so https://github.com/ansible/ansible/pull/45397#issuecomment-466076612 ?
16:54:29 <gundalow> moshloop_: yup, thanks
16:55:32 <dag> The oVirt WG has been started: https://github.com/ansible/community/issues/448, everybody join now ! :)
16:55:43 <bcoca> gundalow: actually, that should not be merged
16:55:53 <bcoca> just on use of str() alone
16:56:15 <gundalow> bcoca: oh, please add a negative review
16:56:17 <bcoca> ^ has not been correctly reviewed
16:56:29 * bcoca adds to list
16:56:42 <bcoca> he, was alrady on my list ...
16:57:12 <gundalow> #undo
16:57:12 <zodbot> Removing item from minutes: ACTION by gundalow at 16:53:51 : merge 24550
16:57:14 <bcoca> gundalow: why merging, i dont see shipits?
16:57:42 <bcoca> does not look like that PR got any full reviews/oks , just a couple of partial ones and no confirmation
16:57:56 <gundalow> oh, must be miss remembering
16:58:23 <ezmac> what caused that to get the docs label?
16:59:26 <gundalow> https://github.com/ansible/ansible/pull/39114 Adding haversine filter - mathstuff.py
16:59:32 <bcoca> -1
16:59:47 <bcoca> feedback kept being ignored
17:03:42 <gundalow> bcoca: where, I see https://github.com/ansible/ansible/pull/39114#issuecomment-383948197 though ends with `but dict also works`, which I assume means that's what they've done
17:04:40 <bcoca> he still didnt fix the interface as i asked
17:04:55 <acozine> bcoca: is https://github.com/ansible/ansible/pull/33740 now symlinking correctly?
17:05:30 <bcoca> lgtm
17:05:53 <acozine> thanks!
17:06:04 * acozine looks back to see if there were other blockers to 33740
17:06:44 <bcoca> acozine: interface needs to be compatible, forgot to mention that, pilou would know that so i didnt add explicitly
17:07:27 <dag> gundalow: there was no "botmeta" label ? I added it now and am adding it to old and existing PRs
17:08:31 <acozine> bcoca: are you saying the interface isn't currently compatible? or that we should check whether it is or not?
17:08:46 <bcoca> latter
17:09:01 <gundalow> dag: OK
17:09:17 <dag> it's too important to miss that a PR changes BOTMETA.yml IMO
17:09:47 <bcoca> acozine: i dont see any review/shipit either, so even then i would not merge until someone did
17:10:40 <gundalow> bcoca: can you please add a `request changes` on 39114, thanks
17:11:15 <bcoca> willdo
17:11:18 <gundalow> https://github.com/ansible/ansible/pull/39174 Passing environmental variable to task definition
17:12:20 <gundalow> hum, why didn't that get aws label
17:12:40 <gundalow> oh, we are on docs PRs now, duh
17:13:17 <acozine> have we looked at https://github.com/ansible/ansible/pull/37080/files?
17:13:22 <acozine> it's not purely docs
17:13:39 <acozine> and my own question is - does anybody want this? does it solve a problem?
17:13:50 <gundalow> https://github.com/ansible/ansible/pull/37080 Environment file feature
17:13:51 <bcoca> -1
17:14:07 <bcoca> i would prefere adding it to config
17:14:14 * bcoca thinks its already there
17:14:53 <bcoca> not the specific feature, but being able to have list of env vars in file
17:15:23 <acozine> bcoca: awesome - is there a line I can point to when closing the PR?
17:15:38 <acozine> or a merged PR that added that ability?
17:16:50 <bcoca> nope, not in config (just checked) but still would not add this way
17:17:02 <bcoca> how is this 'community'? its adding core language feature
17:17:21 <bcoca> labled support:core
17:17:32 <bcoca> why is it being locked at then?
17:17:43 <gundalow> bcoca: we are looking at `is:pr is:open label:docs  sort:created-asc `
17:17:49 <acozine> my goal today is to clear out as much of the backlog as possible
17:17:55 <gundalow> bug fixes was the first 4 hours
17:18:15 <bcoca> ah .. well, these are PRs that happent o have docs, not pure docs PRs, you wont be able to merge most of them
17:18:29 <acozine> it sounds like we can close that one with a note saying "this is not the way we'd like to incorporate this functionality, please open an issue instead"
17:18:53 <bcoca> this would appear in our core backlog meeting
17:18:56 <gundalow> bcoca: Correct, this is about getting some eyes on them. I think we talked about this on Tuesday's Docs W meeting
17:19:15 <acozine> bcoca: yes, exactly - with a broader community participating, including some core folks like yourself, I'm hoping we can take care of some of the code-plus-docs ones
17:19:18 <bcoca> gundalow: i saw that, but didnt realize it included core feature PRs
17:19:42 <gundalow> anyways, lets keep going
17:19:50 <bcoca> this is also why i mentieond we dont have good workflow for 'mixed PRs'
17:20:05 <gundalow> Totally agree, not solving this this week
17:20:19 <acozine> I'm going to close 37080
17:20:24 <gundalow> acozine: +1
17:20:30 <acozine> with note as mentioned above
17:21:55 <bcoca> acozine: dont, let us deal with that in backlog as 'my voice' is not only one needed to make that decision
17:22:49 <acozine> bcoca: do you think there's a chance others will want this approach, and vote to merge this PR?
17:23:12 <bcoca> i think its not likely, but unsure, i get suprised all the time
17:24:52 <acozine> okay, let's see if we can find others to merge or close today
17:26:00 <bcoca> sadly a lot of these are there casue of several 'bad reasons', like  not enough people want it or reject it, no one is interested enough to do full review, etc
17:26:47 <acozine> how about https://github.com/ansible/ansible/pull/39174
17:26:51 <acozine> any docker folks about?
17:27:39 <acozine> hrm, looks like the repo this PR came from is gone
17:27:41 <acozine> not a good sign
17:27:59 <gundalow> acozine: stick on label/candidate_to_close
17:28:08 <cabral404> there's also a comment there without any response, or commit after the comment
17:28:23 <acozine> looking at the actual change, it's just to the docs . . .
17:28:32 <bcoca> that is also why many PRs get stuck 'drive by contribution'
17:28:42 <acozine> erm, change is just to the examples
17:29:02 <acozine> I'm fine with closing those - just prefer not to let them hang around forever
17:30:14 <gundalow> +1 to closing
17:30:29 <acozine> right, closing
17:33:36 <acozine> done
17:34:12 <acozine> hmm, this one's pure docs: https://github.com/ansible/ansible/pull/40073/files
17:34:57 <acozine> any regex experts around?
17:36:20 <gundalow> (in overlapping meetings)
17:36:41 <acozine> meetings, meetings everywhere!
17:37:00 <sivel> acozine: on a general level, it looks correct, on another note, I think the example could be simplified to not rely on `vars`
17:37:19 <sivel> I think the example is overly complicated to convey the problem
17:37:52 <sivel> remove the loop, and use hardcoded values, instead of referencing vars
17:40:05 <acozine> yikes, big chunk of ice just dropped past my window
17:40:29 <acozine> sivel: sounds good, I'll add a comment for the author
17:44:16 <acozine> hm, looking at it again, his point is (I think) that if you want to use one regex to handle both text and integers, it needs to be different from just-text?
17:45:04 <acozine> sivel ^^^
17:45:37 <sivel> yes, but I don't think you need to show it like that.  Yes it must be different to support integers, but that form works for both
17:45:47 <sivel> I think just calling out how to support integers alone is sufficient
17:46:02 <acozine> gotcha
17:47:57 <acozine> how about https://github.com/ansible/ansible/pull/41722
17:50:32 <acozine> hm, I haven't seen this one before
17:50:34 <acozine> https://github.com/ansible/ansible/pull/42775
17:51:00 <acozine> yamllint changes
17:51:09 <acozine> is that an ongoing conversation?
17:52:30 * gundalow looks
17:53:06 <acozine> oh, I see, it's changing some files to address some objections that yamllint raised
17:53:18 <acozine> that's a confusing PR title
17:56:47 <acozine> seems like a combination of useful and harmless changes
17:57:06 <samccann> agreed
17:57:33 <acozine> okay, merging that one then
18:03:36 <acozine> what about https://github.com/ansible/ansible/pull/45692?
18:06:38 <samccann> looks like the contributor did the commit split as requested?
18:06:55 <acozine> yeah, they've responded to comments
18:07:19 <samccann> it's also got code changes tho
18:08:27 <acozine> yeah, it's mostly code changes
18:08:45 <acozine> it's related to a proposal: https://github.com/ansible/proposals/issues/132
18:09:14 <acozine> last comment there was "discussed in IRC meeting and we all agree on proposed feature, comments on implementation in PR incoming"
18:10:12 * acozine needs some lunch
18:10:32 <samccann> ok I can keep going...
18:10:42 <samccann> are there folks still around here to continue some docs PR reviews?
18:10:45 <acozine> samccann: sounds great, thanks
18:10:51 <acozine> #chair
18:10:51 <zodbot> Current chairs: LukasKaemmerling acozine akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 dag gundalow hvtuananh ironfroggy kkao07 mrproper navalkp stroobl themroc winem_
18:12:18 <samccann> ok this is a pure docss pr  on vault - https://github.com/ansible/ansible/pull/43993/files
18:12:29 <samccann> anyone around who understands vault?
18:13:12 <cabral404> samccann: I'm more "lurking" than actively helping (first time around here), but yes
18:13:43 <samccann> thanks cabral404  :-) lurking and learning is valid for sure and welcome!
18:14:07 <cabral404> Thanks!
18:14:08 <alongchamps> I'm doing the same, also in another meeting
18:14:22 <samccann> :-) thanks alongchamps
18:17:38 <cabral404> I've never used multiple vault passwords, but the rest looks good
18:18:02 <samccann> thanks can you put a comment in the PR to say that so we don't lose track?
18:18:15 <alongchamps> same here; as someone relatively new to vault, I appreciate the increase in examples
18:18:25 <samccann> thanks!
18:18:50 <samccann> #action - verify output on https://github.com/ansible/ansible/pull/43993/files and merge
18:18:56 <gundalow> +1
18:20:20 <samccann> This one looks like maybe we just need to push a rebase on the pr ourselves - do you agree?  - https://github.com/ansible/ansible/pull/44986
18:22:26 <gundalow> samccann: yup, I'd just create a fresh branch and copy that line in and reference original PR
18:22:31 <gundalow> samccann: infact I can do that now
18:22:49 <samccann> thanks gundalow!
18:22:50 <gundalow> #action gundalow to create fresh https://github.com/ansible/ansible/pull/44986 and use `C(...)`
18:23:28 <samccann> let's go to this one next - https://github.com/ansible/ansible/pull/46979
18:23:46 <samccann> bcoca: ^^  it looks done and just needds a rebuild_merge?
18:24:00 <samccann> (has 'stale_ci' label)
18:24:13 * samccann doesn't know fancy irc formatting to save her soul
18:24:48 <gundalow> samccann: backticks `
18:25:09 <gundalow> though only important thing is `#action do X`
18:25:21 <samccann> :-) thanks gundalow
18:25:57 <gundalow> 46979: Adding some comments
18:26:54 <samccann> great, thanks!
18:27:21 <gundalow> done
18:27:22 <gundalow> next?
18:27:27 <samccann> next - https://github.com/ansible/ansible/pull/47507
18:27:46 <samccann> I tried to make sense of it, but got to the point where I felt like it wasn't the right patch.. see comment  thread
18:29:38 <gundalow> hum
18:30:02 <gundalow> is the update to `scopes` applicable to everything where https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/doc_fragments/gcp.py#L30 is used?
18:31:26 <samccann> that I don't know.  But I also wonder why the code that 'looks' like it fills in a default value for scopes doesn't seem to work, according to the issue/PR owner?
18:32:02 <samccann> https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/google/gcp_compute_network_facts.py#L152
18:32:07 <gundalow> urgh
18:32:14 <samccann> that was the other thing that stumped me in this PR
18:32:24 <gundalow> (urgh at this as it's autogenerated code)
18:32:42 <gundalow> oh, actually
18:32:49 <gundalow> samccann: move onto the next one, I'll close this
18:32:53 <samccann> ok thanks!
18:33:38 <samccann> half docs half code - https://github.com/ansible/ansible/pull/48090 foreman
18:33:57 <samccann> perhaps still just waiting for comments from some folks? (requested 10 days ago)
18:35:09 <alongchamps> I don't know GCP well enough but I wonder if 'scopes' is required everywhere, in which case it should be in the docs fragment
18:35:17 <alongchamps> if not, then it's a case-by-base basiss
18:36:25 <samccann> thanks alongchamps
18:36:25 <gundalow> FYI a lot of the GCP code is autogenerated https://github.com/GoogleCloudPlatform/magic-modules so I've created the issue upstream
18:36:38 <samccann> ok cool and v. good to know!
18:38:22 <alongchamps> magic-modules is interesting, looks like I have something to explore later
18:39:17 <samccann> ok next one - part code/part docs but it looks like it's waiting for an answer from ansible folks - https://github.com/ansible/ansible/pull/48453
18:39:31 <samccann> maybe someone in the know can put a comment in to answer the PR owner's question?
18:40:08 <gundalow> #action merge 52553
18:42:34 * gundalow looks
18:42:46 <samccann> thanks gundalow!
18:43:36 <samccann> meanwhile - the next one - https://github.com/ansible/ansible/pull/49006
18:44:00 <samccann> looks like it's got code approval. I can review the docs portion and merge if folks agree that's the next step?
18:46:13 <samccann> #action review docs portion and merge 49006
18:46:46 <samccann> here's a biggie!  removing the old srtd theme from the docsite! - https://github.com/ansible/ansible/pull/49289
18:46:58 <gundalow> samccann: I think we skip that one
18:47:01 <samccann> I think it's ready to merge.  I generated locally and it looks good.
18:47:18 <samccann> sorry skip which one gundalow? typing too fast here
18:47:19 <gundalow> samccann: though please remind me about it next week, I'll make some time to do testing
18:47:33 <gundalow> samccann: remove old srtd theme
18:47:58 <samccann> ok should I put WIP on it so nobody merges by mistake?
18:48:12 <gundalow> Well, I mean if people here are interested *please* take a look, though we can review more in the same time
18:48:17 <gundalow> samccann: I'll do that now, good idea
18:48:28 <samccann> kewl thanks
18:49:51 <samccann> gundalow: looks like you grabbed this one for yourself in dec? - https://github.com/ansible/ansible/pull/49705
18:53:05 <samccann> meanwhile for everyone - this could use a few quick eyeballs to verify if the PR owner implemented the fixes bcoca requested - (purely a doc fix) - https://github.com/ansible/ansible/pull/50204
18:53:09 <gundalow> 49006 comments added
18:53:14 <samccann> thanks gundalow!
18:54:09 <gundalow> #action gundalow to create new PR to replace 49705
18:54:12 <gundalow> samccann: thanks
18:58:04 <samccann> anyone have thoughts on 50204? I feel like the owner modified the text to comply w/ bcoca's request, but I want your thoughts before I add it to the list to be merged
19:00:03 <cabral404> samcann: I think so too
19:00:29 <samccann> thanks cabral404
19:01:12 <samccann> #action merge 50204
19:02:05 <samccann> this one looks like maybe if we add a comment to clarify what needs to be changed, maybe it can be moved along - https://github.com/ansible/ansible/pull/50205/files
19:02:54 <samccann> (clarifying how regex works).  There's an existing comment, but I'm personally not sure what it's suggesting... someone more 'in the know' could add a clarifying comment?
19:04:13 <gundalow> samccann: I'd be tempted to say that it needs breaking out into bullet point list of `search`,` regexp`, `match` detailing the what/why/how for each
19:04:46 <samccann> ok please add that as a comment :-)  that way if one of us decides to take it over, we have a starting point
19:04:47 <gundalow> What do you think?
19:06:29 <samccann> yeah that makes sense
19:07:00 <samccann> another one where I can't quite make sense of the requested change - https://github.com/ansible/ansible/pull/50593
19:12:04 <samccann> meanwhile, for those who understand vault - here's another one that looks possibly easy to review?  - https://github.com/ansible/ansible/pull/50958
19:12:24 <samccann> please take a look and add any comments/thoughts to the PR
19:14:26 <gundalow> bcoca: could you please add an example to https://github.com/ansible/ansible/pull/50593
19:14:59 <alongchamps> 50958 looks clear to me on the changes, will add a comment on the PR
19:16:29 <samccann> thanks alongchamps!
19:17:15 <samccann> acozine: looks like this one might be ready to merge?  -https://github.com/ansible/ansible/pull/51351
19:19:05 <acozine> samccann: the changes are good, though I still have a philosophical concern about 51351
19:19:27 <acozine> my concern is, do we need to document something we do not recommend using
19:19:35 <samccann> this next one is more a request to  educate myself - it's a galaxy licensing patch, but I'm not sure how/why it ended up with a docs label? - https://github.com/ansible/ansible/pull/51398
19:19:54 <samccann> thanks acozine... continue with your philosophical debate then ;-)
19:20:30 <acozine> I guess looking at it again, it's only relative path from CWD we don't recommend; relative path from `ansible.cfg` is fine
19:21:01 * acozine satisfies philosophical bent, merges PR
19:22:23 <samccann> great thanks!
19:23:05 <acozine> re: 51398 - where is the licensing stuff on the ansible/ansible side? we should probably match that syntax, even if the available options are different
19:24:09 <samccann> that I dunno. I think the PR owner is a galaxy person
19:25:02 <samccann> gundalow: how long does this PR review go? I'm just chugging along here but I dunno if/when the end time is
19:25:47 <acozine> ah, in the dev docs, we say "license your module under the GPL license (GPLv3 or later)"
19:26:27 <samccann> perhaps add a as a comment/ask if they want to follow ansible/ansible method?
19:26:48 <acozine> yeah, good idea
19:26:49 <samccann> meanwhile fort hose with python/regex experience - here's a quick/easy one (doc fix) - https://github.com/ansible/ansible/pull/51673
19:27:08 <samccann> for those  (fort hose... yeesh)
19:29:48 <samccann> acozine: any progress on checking with vault peeps on this one? - https://github.com/ansible/ansible/pull/51859
19:31:02 <acozine> nope, but all I've done so far is request reviews - I will get more active about following up
19:32:13 <samccann> ok this is another educational one (for me) - has a couple of code approvals... if we approve the docs, do we merge or let the code folks merge?  - https://github.com/ansible/ansible/pull/51404
19:32:21 <samccann> I'm assuming the latter but figured I'd check
19:33:04 <acozine> I'm looking at the regex one, and there's an associated issue: https://github.com/ansible/ansible/issues/45709
19:33:51 <samccann> ok thanks
19:34:00 <samccann> #action - add docs review to https://github.com/ansible/ansible/pull/52355
19:35:35 <acozine> re: 51673/45709 if the issue author is correct, then so is the PR, but I have not tested
19:35:57 <acozine> ^^^ that's the regex remove-the-| one
19:36:02 <gundalow> samccann: till we reach the end of we've had enough
19:37:10 <acozine> re: 51404, that's from a core team member - we should review the docs and then ping the core team back, I suspect we can merge it
19:37:23 <acozine> where "we" == docs team
19:37:34 <samccann> thanks gundalow. acozine: we are at the point now where we are looking at doc PRs we(you and I) haven't looked at yet (last few days).  Do you want to continue?
19:38:07 <acozine> nope, if we've looked at all the backlog PRs, I think we celebrate now
19:38:11 <acozine> that was our goal
19:38:18 <samccann> \o/  party time in docs land!!
19:38:29 <acozine> \o/
19:38:56 <samccann> thank you everyone who's stuck it out this far and helped us move PRs along from the backlog!  you've been a fantastic help!
19:38:56 <acozine> thanks to everyone who lurked, commented, participated, etc.!
19:39:36 <acozine> gundalow: do you want to move to another topic? if not, I can end the meeting
19:40:00 <gundalow> hum
19:40:05 <gundalow> #chair
19:40:05 <zodbot> Current chairs: LukasKaemmerling acozine akasurde alongchamps baptistemm_ bmalynovytch[m] cabral404 dag gundalow hvtuananh ironfroggy kkao07 mrproper navalkp stroobl themroc winem_
19:40:14 <gundalow> Thank you everybody for joining!
19:40:22 <gundalow> #endmeeting