14:01:09 #startmeeting DaWGs Mini Docs PR Review Day 14:01:10 Meeting started Tue Apr 20 14:01:09 2021 UTC. 14:01:10 This meeting is logged and archived in a public location. 14:01:10 The chair is acozine. Information about MeetBot at http://wiki.debian.org/MeetBot. 14:01:10 Useful Commands: #action #agreed #halp #info #idea #link #topic. 14:01:10 The meeting name has been set to 'dawgs_mini_docs_pr_review_day' 14:01:27 woot woot! 14:01:30 #topic goals and ground rules 14:01:51 #info our goal today is to review (and merge if possible) as many docs PRs as possible 14:02:23 o/ 14:02:33 #info ground rules: all are welcome 14:02:42 #chair samccann tadeboro 14:02:42 Current chairs: acozine samccann tadeboro 14:03:08 #info if you want to be a "chair" of the meeting, just wave like this `o/` or say hi 14:03:20 shall we mark each PR we review with the pr_day label so we can go back and track the results? 14:03:27 samccann: yes, great idea 14:03:42 ok I'll take that on as a task as we go along 14:04:58 #info we follow the Ansible Code of Conduct: https://docs.ansible.com/ansible/latest/community/code_of_conduct.html 14:05:53 okay, since andersson007_ suggested some PRs to look at right before the meeting started, let's begin with those 14:06:54 #info PRs from the ansible/ansible repo are welcome, so are PRs from collections repos 14:07:01 let's look at https://github.com/ansible/community-docs/pull/5 14:08:25 do we info the PRs as we look? 14:08:30 samccann: sure 14:08:39 #info https://github.com/ansible/community-docs/pull/5 14:09:27 heh, this PR contains words of wisdom: 14:09:52 `Look through the currently open pull requests in the collection repository. You can help by reviewing them. Reviews help move pull requests to merge state. Some good pull requests cannot be merged only due to a lack of reviews. For more information how to provide a good review, refer to the review checklist. And it is always worth saying that good reviews are often more valuable than pull requests themselves.` 14:09:56 so it may need a copy edit. Should we pick one of us to do that while others to a more substantive review/suggested edits so we aren't clobbering one another? 14:10:15 sure 14:10:23 who wants to do a copyedit? 14:10:26 I see one typo 14:10:54 if no one else hops in, I can do a copyedit while you and others read through more for comprehension, questions, etc 14:11:09 tho now the pressure is on to make sure I find that typo!! 14:11:21 heh 14:11:35 late to the party 14:11:40 #chair lmodemal 14:11:40 Current chairs: acozine lmodemal samccann tadeboro 14:11:44 welcome! 14:12:10 Hmm, I have some questions about the content itself. For example, do we really want to mash together code development and collection development in the quickstart? 14:12:49 tadeboro: good question 14:13:23 I will probably add a reviw to that PR and suggest that we mybe split it in two. 14:13:48 Because that quickstart is anything but quick right now ;) 14:15:11 heh 14:15:17 I was wondering about the title 14:16:09 I might call it "Making your first contribution" or something 14:17:44 we use the words "developer" and "development" to mean so many different things, but this seems to be about getting new contributors up to speed 14:18:05 tadeboro: you're right that for a "quickstart" it's pretty long 14:18:27 And now that I read things through, the purpose is to help people start contributiing to collections. So the docs definitelly need to drop all ansible/ansible git repo stuff because it is irrelevant. 14:19:55 Taking my kid to school today. I may not get back in time for the meeting (if we're not skipping the meeting). 14:20:00 I've just started reading the quickstart - should we change the example away from `community.mysql` to something like `community.example` so there's no risk of clashing with the actual community collection (assuming it either exists or someone may create it)? 14:20:38 Thinking about someone who uses this to learn, forgot they created it, and then end up clashing with the real one a year later or something? Or am I being to docs paranoid ;-) 14:21:31 oh ignore me 14:21:45 I was reading it as creating a collection, not opening a pr.. doh! 14:22:24 abadger1999: drive safe 14:22:45 the meeting today will be the docs review day 14:22:54 and it will go past the regular meeting time 14:23:01 so feel free to join us when you get back 14:23:54 ok another noob question - I don't ever use ssh to when I bring a repo down locally because I did that when I started on ansible/ansible and ended up merging a commit w/o a PR. 14:24:11 woopsie 14:24:26 Should we remove that from the quickstart and just leave the HTTPS option? or is it not possible for a noob like me to do something like that anymore in gitland? 14:25:09 I'd leave just one option, and maybe add a link if there's a good page in the GitHub docs about cloning repos 14:25:19 good catch 14:25:31 The guide uses fork of the repo, so using SSH should be OK. Non-core people cannot break things. 14:26:01 oops, I never changed the topic 14:26:04 #topic https://github.com/ansible/community-docs/pull/5/files 14:26:25 ok gonna leave that for y'all to comment on in the PR directly since there's a difference of opinion there ;-) 14:26:29 tadeboro: I like your optimism and confidence 14:27:18 IIRC, GitHub is phasing out the "push using username and password" functionality, so going HTTPS in instructions might not be the best here. 14:29:25 tadeboro: ah, good to know 14:29:43 I should check how I've got this computer set up 14:31:26 we've got four folks actively reviewing right now 14:31:32 I added my review to the PR. But I think a few more developers/maintainers should go over it and state their oppinion. 14:31:46 yeah, this isn't one we can merge today 14:32:06 for a little variety, how about we look at some one-liners from the ansible/ansible repo? 14:32:39 samccann: are you good to finish out your copyedit review while we look at some other PRs? 14:32:54 tadeboro: thanks for the review! 14:33:18 Yeah, we should probably focus on a more "strictly docs, not a process itself" PRs. 14:33:29 #topic https://github.com/ansible/ansible/pull/74334 14:33:36 yeah go ahead and move on 14:33:48 thx 14:33:59 from the conversation on this one, it looks like the original fix is not the right one 14:34:06 but the docs are inconsisten 14:34:30 s/inconsisten/inconsistent 14:35:11 it should either be `3.7` everywhere, or `3.8` everywhere 14:35:36 I think Matt has this one under control. Not sure what else we can add there. 14:36:25 #topic https://github.com/ansible/ansible/pull/74254 14:36:49 this one looks good to me 14:37:01 we keep making edits to the page and forgetting that some lines are bolded 14:37:21 when you add a new line, you have to update the `emphasis` references 14:38:05 * tadeboro is still counting lines ... ;) 14:38:15 heh 14:38:42 Thinks look OK from my side. 14:39:29 thanks 14:39:31 merged! 14:40:11 #topic https://github.com/ansible/ansible/pull/74330 14:40:19 is this one really a docs PR? 14:40:43 oh, yes, yes it is 14:40:46 Not sure. Maybe the docs part is not ready yet? 14:41:34 Oh, I see the docs are already fixed. And it looks like Brian is taking care of this one. 14:41:46 hm, well sort of 14:42:46 Did the PR miss some parts of the docs that need to be updated? 14:42:55 that is possible 14:43:46 there is one doc change there 14:43:53 rest are tests 14:43:59 welcome Shrews! 14:44:17 yeah, just wondering if there are other places we need to update docs 14:44:19 to match 14:44:20 i want bcoca to look at that again, so you can skip it 14:45:14 not sure if there are more docs, tbh 14:45:40 we've already removed references to the bare `include` param from https://docs.ansible.com/ansible/latest/user_guide/playbooks_reuse.html#re-using-files-and-roles 14:46:14 he, was just starting review 14:46:36 hey bcoca 14:46:43 thanks! 14:47:06 I don't see any references to `include` in the docs about testing 14:47:28 so I think the docs are up to date 14:48:20 how about this one? 14:48:23 #topic https://github.com/ansible/ansible/pull/74288 14:48:29 * samccann finally finished my copyedit from waaay back 14:48:53 thanks samccann! 14:49:03 ah, this PR is for the Docker guide 14:49:36 74288 looks good from the technical side (I had to fix our CI and I know the instructions work as expected). 14:50:07 awesome - looks good from the wordsmithing side too 14:50:24 Hi, I think I found a missmatch between Ansible documentation and comments in code. I created a PR to resolve this (https://github.com/ansible/ansible/pull/73211) maybe you can have a look at this 14:50:48 schurzi: welcome! 14:50:59 #chair schurzi 14:50:59 Current chairs: acozine lmodemal samccann schurzi tadeboro 14:51:07 #chair Shrews bcoca 14:51:07 Current chairs: Shrews acozine bcoca lmodemal samccann schurzi tadeboro 14:51:16 this line reads funny - https://github.com/ansible/ansible/pull/74288/files#diff-0ea0d8cb54d773b8b4eb8a7ac6421c3a02735af2c2a8c5801ad94c385fefd36aR24 14:51:47 `since removed support for Python 2.7` = what removed support for this?? 14:51:49 i can kind of understand what they want to state ... 14:51:54 ah, good catch 14:52:20 did docker remove support? I can add a suggested edit if that's it 14:52:28 yeah, that seems right 14:52:33 go for it 14:52:40 Python package for docker removed support for python 2.7. 14:52:53 ok I'll add that then, thanks! 14:53:18 I think if you add it I can commit it and then we can merge 14:53:33 go for it 14:54:34 done 14:54:43 okay, now let's look at schurzi's PR 14:54:59 #topic https://github.com/ansible/ansible/pull/73211 14:55:08 hello 14:55:23 #chair baptistemm 14:55:23 Current chairs: Shrews acozine baptistemm bcoca lmodemal samccann schurzi tadeboro 14:55:25 welcome! 14:55:38 hey baptistemm! 14:55:43 we are running a docs PR today for the next.. erm.. 3 hrs 14:56:15 schurzi: can you post a link to the comment that says `_text` is deprecated? 14:56:35 The changes in 73211 are correct, but I need a minute or two to bring the VNP up and git grep for other occurences. 14:57:11 ah, found it: https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/_text.py#L7 14:57:45 https://github.com/ansible/ansible/blob/a077bca5d5866603cdc4a2a13aef8f416860aacb/lib/ansible/module_utils/_text.py 14:58:05 schurzi: thanks! 14:58:32 thank you for noticing the deprecation and the fact that the docs were out of date 14:59:18 how do you choose PR ? do you use those with doc or doc-only tags ? 15:00:03 baptistemm: we're looking at any PRs that folks post here, and also at any PRs in https://github.com/ansible/ansible/pulls?q=is%3Apr+is%3Aopen+label%3Adocs+ 15:00:12 If you ave a PR you want us to look at, you can bring that up as well 15:00:49 For 73211, my git grep says it found all occurences. 15:00:49 ah, now is our usual time for the DaWGs meeting 15:00:56 tadeboro: thanks! 15:01:01 woot! shall I approve and merge? 15:01:06 it needs CI 15:01:19 so do a `rebuild_merge` on it 15:01:26 ok thanks! does that still work? 15:01:37 in the azure world? I've forgotten... 15:01:41 I think so . . . let's try it and see 15:02:02 if it hasn't worked in an hour, we can try a different approache 15:02:07 er, approach 15:02:11 done. I'll keep the tab open to check 15:02:11 there is PR of mine https://github.com/ansible/ansible/pull/73984/files if you want 15:02:17 baptistemm: sounds great 15:02:29 can you also answer the question on how to handle all the usages in code? I did a only docs Pr because I don't know how to contunie best from here 15:02:38 74288 that had the tiny edit is passing CI now so imma merge it 15:02:57 awesome, thanks 15:03:16 73211 also has all checks green if I see correctly. 15:03:38 tadeboro: yes, but it's marked `stale_ci` 15:03:45 But they ran a few months ago ... 15:03:53 I just noticed that, yes. 15:04:11 99 times out of 100 that will be fine, but the 100th time . . . 15:04:15 cyb-clock sez we are an hour into this 15:04:22 \o/ 15:04:33 schurzi: to answer your question about the code 15:04:44 how many mentions did you find? 15:05:06 (of `ansible.module_utils._text`) 15:05:30 in ansible repo a few hundred I think, in all community collections uncountable ... 15:05:34 heh 15:05:46 here's what I would suggest 15:06:10 do a PR that updates at most 25 files 15:06:48 you can post it here and in the `ansible-devel` channel 15:06:57 schurzi: I do not know when exactly that change landed, but I would assume collections cannot be updated right now because they usually support a range of Ansible Core versions. 15:07:24 baptistemm - for your PR - there are a few examples that don't have a comment with the expected results, starting here - https://github.com/ansible/ansible/pull/73984/files#diff-a88c795a6652dcd2f429b600ae12d60574d9cc9ac587e43798200a569b8abc17R649 15:07:44 #topic https://github.com/ansible/ansible/pull/73984/files 15:07:45 @tade 15:07:45 is it not possible to add them? I dunno what permutations etc does... 15:08:06 tadeboro that change is very old ... ansible2.8 or older 15:08:31 samccann: indeed I added the FQCNs but forgot the examples... 15:08:48 Then opening PRs in collections is OK since most of then require Ansible >= 2.9 15:08:55 dericcrago: dmsimard aminvakil briantist cyberpear kindlehl madonius Tas-sos zbr you folks talking docs today? 15:09:29 #info this is the usual start time (well, I'm a bit late) for the Docs Working Group aka DaWGs meeting 15:09:45 #info today we are doing a Mini Docs PR Review Day 15:09:56 acozine thanks, i will do that, when I find some time. single PRs per function/plugin seem to be the best approach. Thank you all :) 15:10:08 thanks schurzi !! 15:10:32 schurzi: thank you! join us any Tuesday at this time for the Docs Working Group 15:10:51 and/or chat welcome in the channel any time 15:11:21 samccann: give me 5 minutes I update that 15:11:44 baptistemm: sounds good, post a note when it's ready 15:12:17 #topic https://github.com/ansible/ansible/pull/74268 15:12:32 this one I find very interesting 15:13:07 oh, no, this is a different PR than I thought it was 15:13:09 heh 15:13:42 * acozine looks for the PR she was thinking of 15:14:10 we should still look at this one, though 15:14:32 acozine: samccann: I remember, the output was not legible as it outputs all combinations of permutation ... so that why I did not put that 15:15:12 ah gotcha thanks 15:15:18 I'll approve and merge 15:16:45 I need to take kiddos out or they will trash our appartment. I will start annoying you all again in an hour or two ;) 15:17:28 LOL 15:17:34 thanks tadeboro for the help! 15:18:19 so on this pr, I wonder if `nsible.module_utils.basic._CHECK_ARGUMENT_TYPES_DISPATCHER` being deprecated is something we need to see if it's used anywhere in the docs as well? or does it sound too arcane to be squirrelled away somewhere? 15:20:09 wow.. also, lots of methods being removed and a note about collection owners needing to change. For those of y'all who handle collections, is it enough to have that in the porting guide? 15:20:24 wondering if it needs to go out in the bullhorn as well? 15:21:26 tadeboro: see you later! 15:22:16 * abadger1999 is back 15:22:21 #chair abadger1999 15:22:21 Current chairs: Shrews abadger1999 acozine baptistemm bcoca lmodemal samccann schurzi tadeboro 15:22:31 #unchair tadeboro 15:22:31 Current chairs: Shrews abadger1999 acozine baptistemm bcoca lmodemal samccann schurzi 15:22:38 hmmm this seems a bit of a flag for us in docsland - https://github.com/ansible/ansible/pull/74268/files#diff-55afa0e374c236bc768e486db6ce391902ba4db29f456b34f7c39fafd9913d3bR47 15:22:40 (just while he's out with the kids) 15:23:02 Hmm... From basic? Let's ask sdoran .... a few weeks ago, I commented to him that I would leave the old methods in for a looong time but encourage people to use the new things. 15:23:16 sphinx 3.2???? goodness, we have some upgrading to do 15:23:36 sdoran: ^ see samccann's comments about a lot of module_utils methods being deprecated and collection owners needing to update. 15:25:43 okay, this PR has a lot more than docs in it 15:26:34 yeah just scanned and they already fixed the spelling errors in the code ;-) That's about all I could have added. 15:26:44 heh 15:26:53 we should probably add a comment about the sphinx version 15:27:02 do we have an issue open about upgrading sphinx? 15:27:27 there's probably an old one there somewheres. I can dig it up and put to this PR if you want so we know why it has a new sense of urgency to it ;-) 15:27:40 here's the current one: https://github.com/ansible/ansible/issues/74194 15:29:10 heh, jinx 15:29:36 ok I added a comment. 15:29:59 to that issue and in the PR to ask if there will be display problems for 2.11 because we are older for sphinx 15:31:27 ...and to consider sending some of the info out in the next bullhorn 15:32:38 all right, that might be all we can do on PR 74268 for today 15:32:39 anything else on this PR or should we move on? 15:32:48 moving on . . . 15:32:56 #topic https://github.com/ansible/ansible/pull/74242/files 15:34:50 LGTM 15:35:03 so if I'm reading this right, the inclusive and exclusive notes mean that random sometimes returns 0 but never returns the upper bound number (for example, 347) 15:35:35 yeah. just thinking as well.. dunno if that's a true statement? 15:36:01 me neither 15:36:07 where's the code? 15:37:27 somewhere in here: https://github.com/ansible/ansible/tree/devel/lib/ansible/plugins/filter 15:38:33 * abadger1999 looking 15:39:32 is the random number filter ansible-specific? or is it a standard jinja filter? 15:40:04 I believe it's ansible specific 15:40:42 is it? I thought it was using Python systemRandom() ? 15:40:57 which does seem to be exclusive on the upper number - https://docs.python.org/3/library/random.html#random.SystemRandom 15:40:57 https://jinja.palletsprojects.com/en/2.11.x/templates/#random 15:41:13 I wonder if we overwrite a jinja filter of hte same name... 15:41:23 https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/filter/core.py#L36 15:41:25 https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/filter/core.py#L36 15:41:25 tjat 15:41:29 haha JINX 15:41:30 abadger1999: I opened a PR to add those methods back in, but the decision was made to leave them removed. https://github.com/ansible/ansible/pull/74010#issuecomment-809526310 15:41:33 * acozine can't type 15:41:36 in our docs it says random is an ansible extension of the jinja2 filter 15:42:07 For what it's worth, I went through all the collections in `ansible-community` and look for the use of private methods and opened issues notifying them of the changes. 15:42:35 (sorry to be jumping in the middle of your current discussion — I was in another meeting earlier) 15:42:41 sdoran cool, thanks 15:43:11 the other question was the sphinx version needing to be >=3.2. We are at 2.1.2 right now, is that going to cause display problems when we publish 2.11 docs? 15:44:39 samccann: It works as intended with 2.1.2 because I only have a docstring for just that one private attribute. 15:45:44 But since 2.1.2 ignores the parameters, any docstring added to private attributes will be displayed in the rendered doc. But I don't plan to add any docstrings to other private attributes, so it should be fine. 15:46:09 k thanks for the explanation 15:46:33 for the PR that changes the docs on the `random` filter, the part about it being an integer is correct based on https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/filter/core.py#L238 15:47:07 I don't know how to evaluate the inclusive/exclusive part of the change 15:49:08 oh, and when baptistemm's PR merges, this PR will need rebasing, because they both change the `playbooks_filters.rst` file 15:51:06 yeah not sure either how to determin if hte upper bound is exclusive or not 15:52:15 is it possible to bring it back to the core team for re-review? they took the triage label off, but dunno if that means they agree and left it to docs to merge, or just looked and said it's docs only? 15:53:46 samccann: the docs you posted from the Python systemRandom do show 0 as inclusive and the upper bound as exclusive 15:54:03 and the ansible code does specify integers 15:54:18 yeah but there's another on where it isn't. https://docs.python.org/3/library/random.html#functions-for-integers 15:55:39 we're using the first one (SystemRandom): https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/filter/core.py#L224 15:55:52 we are using both 15:56:07 SystemRandom and Random. Random seems to be used w/ a seed 15:56:10 https://docs.python.org/3/library/random.html#random.Random 15:56:53 true 15:57:03 there's an example with `seed` lower down 15:57:07 in our docs 15:57:20 I think this change is good, though 15:57:33 ok go for it on the merge then 15:57:56 i haven't found my way through the code. But I think you are right it's using SystemRandom (no seed in this example) 15:58:20 samccann: It is correct about inclusive/exclusive 15:58:35 baptistemm: https://github.com/ansible/ansible/pull/73984 is now merged \o/ 15:59:29 ok merged, thanks abadger1999! 15:59:54 cyb-clock sez we are 2 hours into this 16:00:28 halfway mark! 16:01:23 we're going to do a ten-minute break 16:01:34 #topic 10-minute break 16:13:05 acozine: thanks 16:13:40 and we're back! 16:14:02 #topic second half of the first-ever Mini Docs PR Review Day 16:14:32 I found the PR that I was curious about 16:14:38 cool 16:14:40 #topic https://github.com/ansible/ansible/pull/74201/files 16:16:11 it adds instructions on setting "the structure" for the output you're working on - which I have never done 16:16:13 https://github.com/ansible/ansible/pull/74201/files#diff-fdb133ce38964cecbfb4a1ec6fa0d8420eb4dbe627e57f361eab1be9f0cddfb5R125 16:16:20 hmm 16:16:36 sdoran said he ran into this when building the docs locally 16:16:39 yeah that's all part of translation. the foo_structure stuff was added then 16:17:04 you do make coredocs or make webdocs 16:17:59 but do you also have to do `make ansible_structure` or `make core_structure`? 16:18:01 O 16:18:01 or really, make webdocs ANSIBLE_VERSION= no 16:18:14 `htmdocs` will run `ansible_structure` 16:18:31 I don't know if something is now doing that under the covers. I never tested what the structure stuff does. I just know it was added by the translation PR 16:18:33 But in my case, I was just wanting to run `htmlsingle` which complained because no `conf.py` was present. 16:18:37 ahhhh 16:18:57 So I figured out one of the structure commands needs to be run first to symlink in the correct config. 16:18:58 ah yeah, forgot about that one! 16:19:06 so it's necessary if you're building a single page and you haven't build the whole docs at least once beforehand 16:19:08 that makes sense 16:19:14 yes and no 16:19:27 what we really need to do is update the makefiles so htmlsingle can run w/ the correct conf.py 16:19:43 we probably also need to test/fix who to run docs w/ just one module 16:19:55 would we set core as the default, then? 16:20:13 And would it be slow to run the structure target every time? 16:20:17 dunn. haven't thought about it yet 16:20:18 `htmlsingle` is nice because it's fast 16:20:33 yeah, I use it a lot 16:20:51 core doesn't include all the docs so I don't think core would be the default 16:21:33 ansible includes ALMOST all the docs, but again... not all of them. 16:21:34 I meant the default for "I'm just building a single page" 16:21:43 ah, right 16:21:47 again, not without changes 16:22:33 my quick nickel is we open an issue to say htmlsingle and most like the one that will build just one module doc is likely broken. Point to this PR for a 'short term workaround' but not merge the PR 16:22:35 what would happen if I tried to use `htmlsingle` with an rst file that wasn't included in the core build, after running the full core docs build? 16:22:54 dunno never tried it 16:23:08 I'd guess it wouldn't work becfause the file is excluded by sphinx in the conf.py file 16:23:12 sdoran: do you remember what error you ran into? 16:24:04 we could include the extra instructions in this PR as troubleshooting 16:24:21 Sphinx errorer saying no `conf.py` existed. 16:24:34 sdoran: thanks 16:24:52 Just do `make clean` then `make htmlsingle`. 16:25:05 Granted `make clean` is currently broken and won't remove the symlink. 16:25:09 That's the other fix in that PR. 16:25:32 yeah what I could do is take over this PR, figure out the makefile changes etc, and also update the docs section based on that? 16:25:56 Sounds good. 16:26:46 #action samccann to take over https://github.com/ansible/ansible/pull/74201 and update makefiles etc so htmlsingle and making docs w one module works again 16:27:09 sdoran: ah, that explains why I never ran into this error 16:27:23 I'm glad you did, because you will not be the only one, I'm sure 16:29:15 #topic https://github.com/ansible/ansible/pull/74161 16:29:23 finally! an easy one! 16:29:50 let me just double-check that we can merge backports to 2.9 today 16:30:35 cool! gave it an approval 16:31:21 #topic https://github.com/ansible/ansible/pull/74158/files 16:33:10 oops, I have tea steeping 16:33:11 BRB 16:33:17 heh 16:34:53 okay 16:35:03 I have some suggestions for this PR 16:35:30 yeah i'm making some as well 16:36:40 ok submitted my suggested edits 16:36:52 but not sure what the last sentence fragment means or how to fix it. 16:41:40 ok do we batch and commit comments and then merge? or is there anything controversial we should wait on? 16:43:00 bcoca: you around? 16:43:20 no 16:43:28 heh 16:44:36 i'll look after meetinsg 16:44:45 okay, thanks 16:44:45 but feel free to merge with fixes 16:45:40 #topic https://github.com/ansible/ansible/pull/74129/files 16:45:45 this one looks simple 16:47:16 I vote we add a note saying "thanks, did this example trigger an error messages for you? if so, please post it here so other users can find your fix" 16:48:07 I can't tell if it's right or not. I mean the code is there 16:50:06 me neither 16:50:27 hehe well that was a circular journey - went to the kubernetes collection, found the plugin, clicked on teh docs.. and wEEEE back to the same page we are looking at ;-) 16:50:46 heh 16:51:19 akasurde assigned himself to the PR, I expect he'll test it 16:51:26 ok cool 16:52:32 #topic https://github.com/ansible/ansible/pull/74105/files 16:52:55 oof, it needs a rebase 16:53:13 I can do that quickly 16:53:49 cool 16:53:54 i'll reread whilst you do that 16:53:59 thanks 16:59:31 hrm 16:59:36 this rebase is going to get complicated 16:59:58 ok do we wait on it and I can review/merge later? 17:00:08 cyb-clock says we are 3 hours into the meeting 17:00:15 and possiblly talking to ourselves at this point? 17:00:37 so I'm game either way 17:00:46 Hi. I am back for round two. 17:01:42 welcome back! 17:01:46 #chair tadeboro 17:01:46 Current chairs: Shrews abadger1999 acozine baptistemm bcoca lmodemal samccann schurzi tadeboro 17:02:14 samccann: let me know what you think of the current state of that PR 17:02:40 hey tadeboro 17:02:50 we're looking at https://github.com/ansible/ansible/pull/74105/files 17:02:55 tadeboro - any chance you know if this one is correct? https://github.com/ansible/ansible/pull/74129 17:02:58 heh woopsie 17:03:02 BAM BAMABAM! 17:03:04 heh 17:03:07 we can triple-task 17:03:37 * tadeboro looking 17:04:29 acozine I'm happy with the dev guide split. just waiting on the rebase 17:06:26 The 74105 looks correct. Upstream docs with a relevant example: https://kubernetes.io/docs/concepts/workloads/pods/#pod-templates 17:07:32 I'm leaving. From my point of view this PR day was a great idea and the experience as a first time contributor was also great. Thank you all! 17:08:35 schurzi: thanks for coming! 17:10:37 tadeboro: thanks for reviewing 74105 17:11:01 er, sorry, 74129 17:12:24 heh, and we have new docs PRs coming in even as we work! 17:12:30 https://github.com/ansible/ansible/pull/74348 looks like a good addition 17:13:00 #topic https://github.com/ansible/ansible/pull/74088 17:13:57 LGTM gonna merge it 17:14:15 heh, jinx 17:14:18 HAH 17:14:25 * samccann hands acozine her soda back 17:15:50 #topic https://github.com/ansible/ansible/pull/74076 17:16:32 the security objection seems sound to me 17:16:41 yeah sorry forgot to close that one out 17:17:07 excellent! another PR Day win! 17:19:00 #topic https://github.com/ansible/ansible/pull/74087 17:19:04 I'd also vote to close this one 17:20:12 Agreed. 17:20:23 maybe close with a note to say consider opening another PR with a loop example instead? 17:21:21 Looping over services is not something one would do often. 17:21:51 samccann: ah, could you add a comment to that effect? 17:22:05 ah.. then maybe just close it 17:22:05 I closed it already, but the OP will continue to see comments, I think 17:22:18 we're at 100 open docs PRs 17:22:24 one more and we're back in double-digits 17:22:31 woot!! 17:22:38 in the ansible/ansible repo, at least 17:23:21 #topic https://github.com/ansible/ansible/pull/74088 17:23:30 kindlehl put a review on this one already (thanks!) 17:25:07 I also read that one before and it is technically correct. 17:25:54 yeah I'm trying to find evidence on whether the collection has been released with this new platform 17:26:29 I can see it in the repo from 20 days ago, and it was just updated on galaxy I think 5 days ago? 17:26:45 good enough to say yah it's been released? 17:27:06 yeah, that seems good 17:28:04 ok I'll approve and merge 17:28:15 I think that part was not released yet. 17:28:16 \o/ 17:28:22 tadeboro: what part? 17:28:42 https://github.com/ansible-collections/community.network/pull/111 is not part of a release yet. 17:28:42 how can you tell since it was in the repo 20 days ago? 17:29:56 ah it's not on the cliconf plugins in Galaxy, I think you're right! 17:30:01 ahhh 17:30:12 so we're waiting on the next release of community.network 17:30:24 This is a new feature that will be part of the 2.2.0 17:30:38 Patch releases cannot add new features. 17:31:19 which version will go into Ansible 4? 17:31:19 gotcha 17:31:21 phew 17:31:37 do we know yet? 17:32:01 anything up to but not including 3.0.0 17:32:18 https://github.com/ansible-community/ansible-build-data/blob/main/4/ansible-4.build#L40 17:32:30 we can put a note on the ansible/ansible PR that says "merge when community.network 2.2.0 is released" 17:32:39 thanks for the link samccann 17:32:39 you thinking we merge because it's supposed to be part of Ansible 4 and this is devel? 17:32:49 we could, or we could wait 17:32:49 If 2.2.0 is released before Ansible 4.0.0, then yes, else no. 17:33:01 gotcha 17:33:06 then let's put a note on it and wait 17:33:08 this is another reason we need these guides punted from ansible/ansible ;-) 17:33:11 will do 17:33:32 heh, though I guess devel is now 2.12, really, which will be Ansible 5 . . . but let's wait anyway 17:33:47 Moving scenario guides to collections will be one big win for everyone involved. 17:34:29 acozine yeah I thought about that, but unlik devel for ansible/ansible, this functionality isn't actually available even if they download from galaxy 17:34:43 yes and yes 17:34:58 cyb-clock sez we are 3.5 hrs into our 4 hr pr marathon 17:34:59 #topic https://github.com/ansible/ansible/pull/73716/files 17:35:10 One. More. Merge 17:35:46 this one passed CI want me to merge? https://github.com/ansible/ansible/pull/74129 17:36:07 I think rebuild_merge will eventually, but you know... if you want that coveted double digit... 17:36:12 heh, sure 17:36:14 go for it 17:37:19 oh bonus points! this new PR links to ansible 2.4 docs! 17:38:00 samccann: which one? 17:38:30 73716 17:38:35 the one we are reviewing now 17:38:41 oh, in the summary 17:38:45 heh, I'd missed that 17:38:56 https://docs.ansible.com/ansible/latest/reference_appendices/config.html#default-allow-unsafe-lookups is the "modern" alternative with the same info. 17:40:42 yeah, I'm gonna edit the summary 17:40:48 we don't need more references to 2.4 17:41:00 otherwise, though, what do folks think of the change? 17:41:12 The wording in that PR is a bit off since it is not the lookup itself that is marked as unsafe, its return value is. 17:41:46 Regarding 74201, I didn't know about htmlsingle..... there's also singlehtml.... 17:42:00 link updated 17:42:09 'lookup return values are marked unsafe..." ? 17:42:20 as in a suggested edit, then we commit and merge? 17:42:23 Yes, that is the correct variant. 17:42:53 +1 17:43:12 who wants to make the suggested edit? 17:43:15 did it 17:43:21 you wanna commit and merge? 17:43:25 \o/ 17:44:09 committed 17:44:17 as soon as CI runs, I'll merge 17:45:32 sdoran: What does htmlsingle do? 17:46:21 it builds one rst page into html 17:46:51 you pass it `rst=path/to/file.rst` 17:47:14 Ahh, so not even a single plugin... it's a single file. 17:47:34 yep 17:47:36 (And you just mentally ignore all the undefined reference warnings in the output)? 17:47:40 yeah 17:47:47 it's handy when you're testing changes 17:47:53 Okay. I think I can help out with the Makefile change. 17:47:59 abadger1999: awesome! 17:48:07 I'll post a comment to the PR for samccann when I do. 17:48:24 cool thanks abadger1999 17:49:13 okay, we have ten minutes left 17:50:36 Yup. Just builds a single rst file into HTML. 17:54:08 we gonna try for one more pr? 17:55:06 I merged 73716 17:55:10 oh wow... the search we used here doesn't include all of these -https://github.com/ansible/ansible/pulls?q=is%3Aopen+is%3Apr+label%3Adocs_only 17:55:19 as in some are just getting docs_only labels 17:55:31 hm 17:55:55 sorry this is a better one - https://github.com/ansible/ansible/pulls?q=is%3Aopen+is%3Apr+label%3Adocs_only+-label%3Adocs 17:56:06 only 4 have the `docs_only` but no `docs` label 17:56:18 https://github.com/ansible/ansible/pull/73273/files looks like an easy one. 17:56:27 yeah, that's a trend we need to think about - is that something we want? 17:56:56 #topic https://github.com/ansible/ansible/pull/73273/files 17:56:57 tadeboro: nice 17:57:45 what do we think about baptistemm's comment about making those references links? 17:57:50 does it? haha 17:58:37 i don't know if sphinx makes them links or not. I'd guess it does 17:58:41 I can add suggestions easily enough 17:58:46 I don't think so, no 17:59:15 I would say we convert those to explicit links and commit the changes. 17:59:22 yeah sounds good 17:59:40 acozine you wanna do it and I'll merge this time? 18:00:51 https://github.com/ansible/ansible/pull/73273#pullrequestreview-640276535 18:01:06 samccann: go for it 18:02:26 ok waiting on CI 18:02:34 and let's count how many PRs we've done 18:03:50 11 PRs merged, 7 additional PRs reviewed 18:04:01 tho one of those will go to the merged pile soon! 18:04:14 I count 12 18:04:21 #info merging 12 PRs and reviewed additional 7 PRs 18:04:34 not bad... not bad at all 18:04:57 To what pile the closed ones go? Or was just one that we closed without merging? 18:05:58 ah sorry yeah the 'merged' included one closed PR 18:06:06 #undo 18:06:06 Removing item from minutes: INFO by samccann at 18:04:21 : merging 12 PRs and reviewed additional 7 PRs 18:06:22 #info 11 PRs merged, 1 closed, 7 reviewed/requested changes 18:06:33 ok we ready to close this meeting out? 18:06:42 I think so 18:06:47 I need some lunch! 18:07:12 thanks samccann, abadger1999 and tadeboro for sticking around to the end! 18:07:31 Thanks everyone who brought a PR to the group, and to everyone who reviewed a PR 18:07:52 please post any suggestions on how to improve the experience to the chat 18:07:57 #endmeeting