19:00:53 #startmeeting Ansible Community Meeting 19:00:53 Meeting started Wed Jan 13 19:00:53 2021 UTC. 19:00:53 This meeting is logged and archived in a public location. 19:00:53 The chair is felixfontein. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:00:53 Useful Commands: #action #agreed #halp #info #idea #link #topic. 19:00:53 The meeting name has been set to 'ansible_community_meeting' 19:00:53 #topic Agenda https://github.com/ansible/community/issues/539 19:00:53 abadger1999 acozine andersson007_ baptistemm bcoca briantist cyberpear cybette dericcrago dmsimard felixfontein geerlingguy gundalow gwmngilfen ikhan_ jillr jtanner lmodemal misc nitzmahone resmo samccann tadeboro: ping! 19:00:57 #topic Updates 19:00:58 o/ 19:01:03 * dericcrago waves 19:01:03 o/ 19:01:05 Hello 19:01:07 #chair dmsimard dericcrago cybette abadger1999 19:01:07 Current chairs: abadger1999 cybette dericcrago dmsimard felixfontein 19:01:19 o/ 19:01:30 #chair acozine 19:01:30 Current chairs: abadger1999 acozine cybette dericcrago dmsimard felixfontein 19:02:02 Really quick announcement: The core team is working on schedules for future ansible-base/ansible-core releases. 19:02:32 It hasn't been finalized yet but this is what they're thinking of doing 19:02:37 \o 19:02:37 moving to a hard set 6 month -core release timeline April/October 19:02:43 #chair samccann 19:02:43 Current chairs: abadger1999 acozine cybette dericcrago dmsimard felixfontein samccann 19:02:45 ansible-core releases slightly before equivalent ansible automation platform releases. 19:02:57 ansible-2.9 will EOL in correlation with the ansible-core-2.15 release based on this timeline 19:03:03 ansible-base-2.10 will be going EOL April 2022 19:03:31 At some future point we'll need to think about how we want that to affect our schedule for the ansible package. 19:03:48 But we can wait until core has finalized this as their schedule 19:04:00 my personal opinion: we should synchronize so that we release after ansible-core releases 19:04:10 +1 for waiting :) 19:04:30 #info https://github.com/ansible/community/issues/539#issuecomment-758845140 19:04:39 (that's essentially the same what abadger1999 said above) 19:05:15 #info reviewing new collections for Ansible 3.0.0 has begun! 19:05:22 \o/ 19:06:29 o/ 19:06:34 has anyone else an announcement to make? 19:06:36 felixfontein: how many new candidates do we have? 19:06:36 #chair jillr 19:06:36 Current chairs: abadger1999 acozine cybette dericcrago dmsimard felixfontein jillr samccann 19:06:53 acozine: 5 as per https://github.com/ansible-collections/ansible-inclusion/discussions 19:06:54 acozine: right now, there are five 19:06:55 how many potential new collections in Ansible, I mean 19:07:02 that's great 19:07:34 I'm happy it's not 20 or so ;) 19:07:37 yep 19:07:42 not an overwhelming number 19:07:47 but not "crickets" either 19:08:34 from a docs perspective, at what point should we 'test' the docs pipline that it works with potential new collections? or is there/will there be/ some type of CI to verify 'it all works'? 19:09:12 samccann: basically right now, `ansible-test sanity` does most of the work. resp. antsibull-docs should not create output that breaks the docsite 19:09:28 ok cool, thanks! 19:09:52 In my experience as a candidate for inclusion, if sanity passes, docs render into rst just fine. 19:09:59 samccann: yes as felixfontein mentioned as long as ansible-test sanity is good. Everything should fall in place 19:10:08 #chair ikhan tadeboro 19:10:08 Current chairs: abadger1999 acozine cybette dericcrago dmsimard felixfontein ikhan jillr samccann tadeboro 19:10:25 we are 10 minutes into "Updates" 19:10:29 in the worst place antsibull-docs will generate an error page instead of a docs page for some module/plugin :) 19:10:49 ok, since there are no more updates, let's continue 19:10:49 #topic Collection dependency: avoid unnecessarily strict requirements to avoid dependency hell for Ansible building? 19:11:17 we started discussing collection dependencies last week, and added a requirement that collections in Ansible must not add collections outside of Ansible as a dependency 19:11:58 I think it would also be good if we require collections to avoid "unnecessary" dependencies (in the sense of being to strict with version specifications), since that can result in a lot of trouble for building a consistent Ansible package 19:12:12 I like that too. 19:12:13 felixfontein: how that will be handled if collection that is dependent on other collection is tagged for deprecation? 19:12:17 like when community.foo requires community.bar < 2.0.0, and community.bar releases 2.0.0, we have a problem 19:12:46 ikhan: do you mean that the collection which depends on the other is deprecated, or the collection the other depends on is deprecated? 19:13:29 I don't think we talked about deprecating collections yet, at all (at least w.r.t. the Ansible package) 19:13:30 felixfontein: I am talking about the later scenario that you mentioned 19:13:49 ikhan: do you have a specific collection in mind? 19:14:20 I guess we'd have to keep the deprecated one around until all other collections in Ansible stop depending on it 19:14:35 I guess it also depends on how the collection implements its deprecation 19:14:43 felixfontein: not really - just trying to understand if we have a risk of running in that scenario if we are not planning to deprecate collection then this might be a moot point 19:15:41 Is it even possible to deprecate a whole collection? 19:15:45 this problem will probably appear at some point, like when community.kubernetes is renamed to kubernetes.core - we a) have a collection which depends on community.kubernetes (namely community.kubevirt), and b) users might use community.kubernetes in their playbooks/roles and should be informed in some way 19:16:13 tadeboro: I guess what one could do is deprecate every single plugin and module in that collection. (no idea how that works for roles and playbooks though.) 19:16:38 felixfontein: I was thinking about the same scenario 19:17:12 I still have no idea what the timeline of that rename / deprecation is though, so maybe we can ignore it for now ;) 19:17:33 or start thinking once it has been decided what happens with community.kubernetes 19:18:03 the last thing I heard was that not much is known officially 19:18:18 I think at some point we'll have to address that. To do it well, we need to (1) build up the communication mechanisms we have with the collection owners and (2) we write down a policy for what happens in that case. We probably want a policy that allows another collection to take over a deprecated collection but maybe, given that we ship upstreams directly, the best we can do is allow for compatible collections that 19:18:19 people have to manually switch to depending on 19:19:01 it's probably best to keep a 'dead' community.kubernetes that has deprecated redirects, at least for some time 19:19:30 so that existing roles/playbooks keep working, but will get deprecation warnings which they can fix by updating their FQCNs 19:20:28 * gundalow waves 19:20:31 but back to the "unnecessary" dependencies. I'm not sure how we can specify that in a way that is understandable and enforceable for the collection requirements 19:20:32 we're 10 minutes into "Collection dependency" (and 20min in the meeting) 19:20:32 I think we have some time to think this through 19:20:34 #chair gundalow 19:20:34 Current chairs: abadger1999 acozine cybette dericcrago dmsimard felixfontein gundalow ikhan jillr samccann tadeboro 19:21:05 should we postpone the dependency topic until someone has a concrete suggestion? 19:21:59 that's probably for the best 19:22:13 #info we have to think how deprecation of collections will work (example: community.kubernetes -> kubernetes.core rename) 19:22:20 I believe the intention is to retain redirects for some time but we don't have a full task outline for this work right now 19:22:28 #info postpone dependency discussion until someone has a concrete suggestion 19:22:58 jillr: if there's something more concrete, please keep us posted :) 19:23:25 It is also problematic since ansible-core does not want to keep updating redirects, which means removal of collection could break the redirect chain. 19:23:28 felixfontein: that will all go into 2.0 planning, which should make it to the project board 19:23:37 ok, now I prepared some topics from points that individual reviewers suggested (dmsimard, tadeboro, me) 19:23:55 tadeboro: we have one more update, so if they decide quickly enough, we can update it for ansible-core 2.11 19:24:33 jillr: do you have a link to the project board? (there are so many project boards, I don't know which one you mean :) ) 19:24:59 felixfontein: oh sorry! yeah I do 19:25:02 #link https://github.com/ansible-collections/community.kubernetes/projects/2 19:25:07 tadeboro: I'm collecting updates in https://github.com/ansible/ansible/pull/73046 19:25:14 jillr: thanks! :) 19:25:18 #topic Should DOCUMENTATION, EXAMPLES, RETURN be required? (dmsimard) 19:25:26 o/ 19:25:48 dmsimard: do you want to sum your point from https://github.com/ansible/community/issues/539#issuecomment-756165507 up? 19:26:08 it is also related to my comment on documenting RETURN values 19:26:08 We came across plugins that had DOCUMENTATION and EXAMPLES but no RETURN and I suppose we could also come across plugins without EXAMPLES or DOCUMENTATION 19:26:40 The collection requirements has guidelines about these but they are not explicitly described as required/mandatory 19:26:41 I'm not certain I like those, especially return. 19:26:45 it could be that DOCUMENTATION is required by ansible-doc, I haven't tried that 19:26:51 some older stuff is like that. (missing RETURN, plugins with nothing) 19:27:04 also validate-modules resp. my plugin PR will make sure that DOCUMENTATION is around for all plugins and modules :) 19:27:07 but i'd like it to be a hard requirement for anything new 19:27:09 Sometimes bad documentation is worse than no documentation and return easily gets out of sync with what is actually returned 19:27:29 yeah, DOCUMENTATION feels like it is a requirement 19:27:40 the problem with RETURN is that there's no good way to sanity test whether it makes sense - as opposed to DOCUMENTATION (which is compared to the argument spec for modules) 19:27:53 also RETURN does not make sense for all plugin types 19:27:58 there is something in test that verifies docs match argspec now... is there anything similar that could be done for RETURN so it can't get out of sync? 19:28:08 samccann: not that I can think of. 19:28:16 that's part of the problem. 19:28:18 it's pretty much impossible 19:28:22 ah sorry yeah felixfontein answered it 19:28:30 return is quite dynamic, we wouldn't be able to test it without running the module with a variety of arguments 19:29:03 and even then we can never be sure 19:29:08 if we designed a framework from scratch it could be possible but annoying. (define a return spec. If, when *the user* runs the module, the return doesn't match the return spec, throw an error) 19:29:28 abadger1999: that would lead to a lot of module crashes :) 19:29:35 I'd vote to leave RETURN docs as optional 19:29:39 Yeah, really, really annoying. 19:29:54 I don't think I've ever reviewed `RETURN` matches the code for any module in much detail. I have reviewed the wording for the details in the RETURN docs block 19:30:00 ok so if we have a bad RETURN section, then what happens? user gets confused/frustrated.. opens an issue (maybe even a PR) and it gets fixed 19:30:11 I think modules/plugins should document important RETURN values. for example an _info module without RETURN docs is worthless 19:30:11 if we have NO RETURN section... what's that impact on the end user? 19:30:18 we've added blank `RETURN` sections to some modules recently 19:30:30 I can't remember if anything shows up in the docs for those 19:30:42 felixfontein: Heh.... the setup module ;-) 19:30:57 ok so in the best interest of time since we have other topics, let's vote ? Make DOCUMENTATION section in plugins required in collection requirements, declare EXAMPLES and RETURN as optional but nice to have 19:31:11 IMO if the module returns something that users should use, return values should better be documented. if the module returns mainly things that help debugging, but that are seldom or never used, I guess it would be OK to skip the RETURN docs 19:31:12 -1 on not having EXAMPLES 19:31:25 I think the documentation for that says "run ansible -m setup to see what the return value is" 19:31:30 I would vote for DOCS + EXAMPLES always, RETURN if it makes sense. 19:31:37 as in EXAMPLES should be required. 19:31:37 EXAMPLES might not make sense for every plugin type as well 19:31:40 yes, examples are the most frequently used parts of the documentation 19:31:46 I also think EXAMPLES are a reasonable requirement 19:32:08 I often skip results section and just debug the result "live". 19:32:13 ok so the only one optional would be RETURN ? 19:32:25 like for strategy plugins, I'm not sure if EXAMPLES is useful 19:32:30 if EXAMPLES doesn't make sense, then maybe we allow blank EXAMPLES, but I think the requirements say you MUST have examples 19:32:54 samccann: which requirements are you referring to ? we are discussing the lack of such requirement right now 19:32:55 could we CI examples anyway? other than to say the section exists? 19:33:01 yeah if it doesn't make any sense we can make exceptions but the default should be required 19:33:05 dmsimard the new requirements :-) 19:33:29 or cliconf plugins. they usually have only DOCUMENTATION. 19:33:51 I would expect that even for things such as strategy plugin, there should be an example ansible run and explanation of the result/output. 19:33:57 felixfontein: I guess we can be granular in the requirement and specify modules/action plugins 19:34:04 We do the same thing with inventory plugins. 19:34:19 samccann: I think it only says what EXAMPLES *must* do, but not *must* have EXAMPLES 19:35:16 tadeboro: depends on how complex the plugin is. maybe a few lines of explanations in the plugin description already describe everything the user needs to know. but no idea, I haven't really looked at strategy plugins yet ;) 19:35:21 dmsimard: so, yes, RETURN would be the only optional one 19:35:31 we're 10 minutes into topic "Should DOCUMENTATION, EXAMPLES, RETURN be required?" (and 35min in the meeting) 19:35:32 Also, our documentation has examples at the top of the reference docs because we learned that users really like copy-paste-modify approach of authoring playbooks. 19:36:00 yep, most people don't read the parameter docs, they just adapt the examples 19:36:06 ok, we should condense this to something we can vote on 19:36:16 yeah I confess i'm a jump to examples kind of reader myself 19:36:40 felixfontein - maybe we vote on each one separately to capture the different views 19:36:56 idea: DOCUMENTATION is required, EXAMPLES is required, RETURN is required for modules/action plugins, lookup plugins 19:37:27 does anyone want DOCUMENTATION and/or EXAMPLES not required? 19:37:35 if not, let's vote on that part first 19:37:52 +1 for DOCUMENTATION and EXAMPLES required 19:37:58 +1 for DOCUMENTATION and EXAMPLES, 19:38:13 +1 DOCS + EXAMPLES required 19:38:13 hmm, EXAMPLES for cliconf really makes no sense IMO 19:38:31 Are we over thinking this? 19:38:44 heh, probably 19:38:48 Documentation MUST be included. 19:39:09 there are so many plugin types around that most people never saw all of them in action 19:39:13 +1 for DOCS and EXAMPLES 19:39:19 +1 requiring DOCS and EXAMPLES 19:39:22 +1 for DOCUMENTATION and EXAMPLES required 19:39:34 I'm -1 on requiring EXAMPLES, because I'm thinking of cliconf plugins right now 19:39:35 +1 DOCUMENTATION, Maybe EXAMPLES is required except for $list of plugin types? 19:39:47 felixfontein: abadger1999 beat me to it 19:39:56 abadger1999: shouldn't we have a list before we vote? 19:40:06 +1 DOCUMENTATION, Maybe EXAMPLES is required except for $list of plugin types? 19:40:07 how do you use a cliconf plugin? Can you give...heh... and example? ;-) 19:40:13 :D 19:40:14 I'm fine with a list of exceptions where it doesn't make sense, once we have such a list 19:40:23 felixfontein: yeah, I'm +1 for DOCUMENTATION but not voting on Examples yet ;-) 19:40:25 samccann++ 19:40:27 We can work out the details in a PR if it's agreed on 19:40:32 would someone have some time to go through all documentable plugin types until next week and compile a list of plugin types where examples/return values can make no sense? 19:40:34 I'm happy if the EXAMPLE is just text explaining it #this is how you use this blablabla foo foo foo and ladee da 19:40:52 then we can compose a concrete list of exceptions next week, and vote on the result 19:41:04 a) does that sound good, and b) does anyone volunteer? :) 19:41:08 I'll take action on that 19:41:14 IIRC, cliconf modules are used by people programming modules? 19:41:26 #action dmsimard to propose a PR for clarifying requirements for DOCUMENTATION/EXAMPLES/RETURN 19:41:31 so they're invisible to end users? 19:41:32 abadger1999: I think they are used when using certain connection types 19:41:35 #undo 19:41:35 Removing item from minutes: ACTION by dmsimard at 19:41:26 : dmsimard to propose a PR for clarifying requirements for DOCUMENTATION/EXAMPLES/RETURN 19:41:43 #action dmsimard to propose a PR for clarifying requirements for DOCUMENTATION/EXAMPLES/RETURN so we can vote on it next meeting 19:41:54 like the community.routeros collection has a cliconf plugin (which only has DOCUMENTATION, and I don't see what EXAMPLES could be there) 19:42:16 sounds good :) 19:42:31 btw, related issue: https://github.com/ansible/ansible/issues/71793 19:43:09 #topic Should CoC be present in the repository, or is a link to the CoC enough? (tadeboro) 19:43:22 this question was brought up by tadeboro since some collections do not contain the text of the CoC 19:43:37 felixfontein: I had other points that I edited not long before the meeting -- might need a refresh but we can circle back to them later it's ok 19:43:49 jillr: cybette 19:43:51 in fact we don't do that in the collection template, and we don't do that in all the community collections I'm working on - we all just link to Ansible's CoC 19:44:06 I think a link is better than a copy - that way we only maintain it in one place, and there's no opportunity for drift 19:44:18 I had it as a link so if/when Ansible's CoC changes I don't have anything to do 19:44:22 dmsimard: I'm not going through in order :) 19:44:23 The thing that I was not sure about was the lack of CoC file. Official template has a CoC file that contains a link. 19:44:35 A clearly labeled link is fine IMO 19:44:44 ansible.utils has link in README, but no CoC file. 19:45:09 should we clarify in the requirements that a link is fine, or do you think the requirements are fine as-is? 19:45:17 I believe a link is sufficient though we can clarify the verbiage in the collection reqs if necessary 19:45:19 I'd say that's a bug in ansible.utils. a CoC file IMHO is needed so GitHub has links 19:45:21 trying to maintain a copy of the Ansible CoC in dozens of repos is just asking for inconsistencies 19:45:47 gundalow: it's a github convention though, like -- I don't think gitlab or gitea would do the same, for example 19:45:53 gundalow: in that case we should require the exact filename, and say that it can contain a link 19:46:04 gundalow: so you would prefer to have a CoC.md (or alike) that contains the link back to the official copy of the CoC? 19:46:05 And I did not block the inclusion on this, since ansible.utils clearly explains what CoC one should use. 19:46:06 (if we want that) 19:46:09 If the Ansible CoC isn't even versioned (like licenses now are), it will be hard maintaining them in different repos too. 19:46:22 jillr: This is how collection template repo is set up. 19:46:40 Yup, GitHub convention, though I like it https://github.com/ansible-collections/collection_template/community 19:46:56 https://github.com/ansible-collections/collection_template/blob/main/CODE_OF_CONDUCT.md 19:47:00 +1 19:47:01 we have some collections that were created I think before the template was so we need to get those caught up 19:47:17 but should we require this? 19:47:26 so the question is about whether a link is sufficient or should we make a file mandatory 19:47:49 Link is sufficient, but does it need to be in some dedicated CoC file? 19:47:55 it certainly makes it easier for us to ensure that the collection has a CoC, vs having to dig search the README or wherever 19:48:10 I got used to find CoC files in repos in last few years. 19:48:10 I think it needs to be `./CODE_OF_CONDUCT.md` 19:48:15 requiring that it is mentioned in README.md would also be a variant. or require both. 19:48:26 for those working in many different git* projects - are you used to seeing a separate CoC file? 19:48:43 I personally want a dedicated file, even if it's just https://raw.githubusercontent.com/ansible-collections/.github/main/CODE_OF_CONDUCT.md 19:48:51 samccann: in my experience it's a github-ism 19:49:06 i also personally like the github-ism of having a dedicated file for it 19:49:07 ansible/ansible does not have a dedicated file 19:49:13 how about requiring both CODE_OF_CONDUCT.md (which can just be a link), and a link to either this file or the CoC in README.md? 19:49:24 I think a dedicated file is preferrable, but link in README.md is ok 19:49:40 I would say that most repos created recently (are only a few years old) have CODE_OF_CONDUCT.md file 19:49:49 if we require both, we both make it easy to find for galaxy users (they only see the README) and for people browing the GH repo 19:50:31 that sounds good 19:50:46 jillr: it's in the `.github` subdir https://github.com/ansible/ansible/blob/devel/.github/CODE_OF_CONDUCT.md 19:51:01 VOTE: a) require link in README.md, b) require CODE_OF_CONDUCT.md file, a+b) require both, c) require at least one of them 19:51:08 gundalow: ah, I never would have looked there for it 19:51:26 jillr: GitHub ism. That's why i think it should be required in root 19:51:36 a+b 19:51:39 a+b 19:51:40 +1 c, fine with any option though 19:51:46 c 19:52:03 c 19:52:17 a+b 19:52:24 c 19:52:45 a+b 19:53:18 we're 10 minutes into "CoC present in repo or just a link" (and 53min in the meeting!!) 19:53:27 how time does fly! 19:53:38 cybette: thanks for keeping time :) 19:53:41 right now we have 4 x a+b, and 4 x c 19:54:06 #chair 19:54:06 Current chairs: abadger1999 acozine cybette dericcrago dmsimard felixfontein gundalow ikhan jillr samccann tadeboro 19:54:28 who didn't vote... you have a chance to CHANGE THE WORLD! 19:54:31 ;-) 19:54:41 :D 19:54:50 11 voters, 8 votes so far 19:54:50 c 19:55:03 a tie breaker! 19:55:19 excellent 19:55:22 :) 19:55:46 I wouldn't wail and gnash teeth if C wins 19:55:50 #agreed We require at least one of: link in README.md, file CODE_OF_CONDUCT.md (can have link or content of CoC) 19:55:54 (5 x c, 4 x a+b) 19:56:19 we can revisit if this turns out to be particularly problematic or anything 19:56:33 :+1: 19:56:56 #topic Differentiating between MUST change (blocker) to SHOULD change (nice to have, not a blocker) when providing feedback to applicants (dmsimard) 19:57:45 this is me putting myself in the shoes of a collection applicant and getting a bunch of feedback and not knowing what is a blocker and what isn't 19:59:11 for example, I went through t_systems_mms.icinga_director ( https://github.com/ansible-collections/ansible-inclusion/discussions/5 ) which seemed generally OK at my first glance but then felixfontein came and found a lot of things to fix 19:59:14 is this more about the review process (i.e. reviewers should mention in their comments whether it is MUST or SHOULD), or about the guidelines? 19:59:42 When I did a review, I had 4 categories of responses on checklist: only tick - all OK, tick + comment - nice to have fix, but still OK, no tick + comment - must fix, no tick - did not check yet. 20:00:44 felixfontein: it's more about how we provide comments in the disucssion 20:00:49 should be add some text like that at the top of the checklist file? 20:01:04 felixfontein: at a minimum, probably 20:01:16 so that reviewers can use tadeboro's scheme (if they don't decide to replace it by another one)? 20:01:35 In Fedora, for our checklist, we had each item start with either *MUST* or *SHOULD*. When I did reviews, if I made a comment outside of hte review checklist, I'd try to remember to say whether I would block inclusion on the issue, wouldn't approve, or it was cosmetic and I'd approve once the maintainer told me why they were or were not going to fix it. 20:01:36 So are you proposing that when providing written feedback we should use the words `MUST` and `SHOULD` + $details 20:01:39 https://github.com/ansible-collections/ansible-inclusion/discussions/4#discussioncomment-270408 is how I started. 20:02:33 (block inclusion would be... veto if some other reviewer disagreed; wouldn't approve would be I wouldn't approve it like that but if you could find someone else to approve it, I wouldn't hold it up) 20:03:19 tadeboro: I like your system. 20:03:46 it's a bit limiting if you have different comments for one topic, where some are SHOULD and some are MUST 20:03:50 * samccann needs to disappear for 30 min 20:04:08 felixfontein: don't mean to put you on the spot but even if just for personal learning, is there anything in https://github.com/ansible-collections/ansible-inclusion/discussions/5#discussioncomment-272236 that you would consider a blocker ? 20:04:09 samccann: thanks, enjoy 20:06:02 dmsimard: a blocker for me is skipping most of the sanity tests by restricting to only two python versions 20:06:16 dmsimard: the rest are either SHOULD, MUST, or 'wouldn't approve' 20:06:44 we're 10 minutes into "Differentiating between 'MUST' and 'SHOULD' change in feedback to applicants" (and 66min in the meeting) 20:07:06 fair enough 20:07:26 fwiw I see MUST/wouldn't approve/blocker as the same thing ;) 20:07:38 having a consistent scheme of labelling comments as SHOULD, MUST, WON'T APPROVE, and BLOCKER would be good though. both for reviewers and for applicants 20:07:44 +1 20:08:16 hmm now that you mention it, I'm not really sure how to distinguish MUST and WON'T APPROVE 20:08:39 but BLOCKER would be a veto, as opposed to "if someone else approves, I can live with it" 20:08:52 (as abadger1999 wrote above) 20:09:13 abadger1999: do you think we need to distinguish between MUST and WON'T APPROVE? 20:09:45 maybe just having SHOULD, MUST, BLOCKER is good enough 20:10:01 any other opinions on this? 20:10:13 BLOCKER for me is a MUST criteria that isn't met but can be met/fixed 20:11:16 In the checklist itself, there's really only MUST and SHOULD. In reviews, it just needs to be clear what the reviewer is asking for. 20:11:24 does that mean that WON'T APPROVE is an unfixable criteria? I'm trying to think of possible examples of something like that 20:11:59 jillr: I think everything is fixable - it might require serious changes or rewriting a large chunk, but in the end you can always fix things 20:12:03 Some reviewers might block not approve if a checklist SHOULD isn't address, for instance, while other rviewers might decide to approve it anyways 20:12:22 ah, so WON'T APPROVE would be between SHOULD and MUST? 20:12:42 I think they're just different concepts and maybe we shouldn't mix the terminology. 20:12:44 abadger1999: ah yeah ok that makes sense; so a WONT APPROVE is primarily on SHOULDs then? 20:13:05 jillr: That'st he way I would generally use it. 20:13:16 I'm good with that 20:13:22 me too, it sounds reasonable 20:13:42 is there a difference between MUST and BLOCKER then? 20:14:21 I don't think -- though I believe what we're saying is that we should make it obvious to the applicant when something is a blocker (i.e, a MUST that isn't met) 20:14:29 I would say no. MUST and SHOULD are IMHO plenty to work with. 20:14:57 I think the others are mainly relevant when there are conflicting opinions from the reviewers 20:15:07 but yeah, SHOULD and MUST are already a good start 20:15:24 someone want to take the action to review and adjust the checklist ? 20:15:26 I think MUST and SHOULD are adequete, unless we find with time situations where we need to do something more 20:16:02 Yeah, the only difficulty is that the checklist uses MUST and SHOULD but that's slightly different than what a reviewer wants to say sometimes. 20:16:38 I guess that for now, reviewers must express more details with free text then :) 20:16:54 should we VOTE on making it mandatory to use SHOULD and MUST for review comments? 20:16:59 like I say, the checklist might say something like `SHOULD: include a comment when the code is convoluted.` A reviewer might read some really crazy code in this and say `This must be changed or I won't approve it` 20:17:05 we're 20 minutes into "Differentiating between 'MUST' and 'SHOULD' change in feedback to applicants" (and 76min in the meeting) 20:17:12 which might equate to a MUST 20:17:34 so then the user sees: SHOULD: include a comment when the code is convoluted; you MUST fix this. 20:17:42 We can always update this in the future once we've got more feedback 20:17:54 That's why I think different terminology might be good. 20:19:45 would tadeboro's system of checking the boxes with comments (or not) and maybe an explanation of that suffice? 20:19:49 need to step away momentarily 20:20:28 dericcrago: I think it does not suffice 20:20:38 dericcrago: like when having multiple comments for one checkbox 20:20:46 some might be SHOULD, some MUST 20:21:05 I think it's better to be explicit and always mention SHOULD and MUST at the beginning of every comment 20:22:30 maybe use `required` / `optional` if the terminology between the checklist and the comment is confusing? 20:22:47 ok, we should either now vote on something, or we switch topics 20:23:06 I agree with listing whether the comment needs t o be addressed or is optional. I would just liek to use different terms than MUST and SHOULD which are already used. 20:23:29 would required optional be okay, I can get behind that. 20:23:41 if we want a quick vote 20:24:03 VOTE: comments must say whether they need to be addresser, or are optional. (How to precisely do that can be discussed next time.) 20:24:12 *d 20:24:13 +1 20:24:15 +1 20:24:15 +1 20:24:18 +1 20:24:24 +1 20:24:28 thanks dericcrago for example terms 20:24:39 #agreed Comments must say whether they need to be addressed, or are optional. 20:24:52 if someone wants to prepare a proposal for next time, paste it in the agenda :) 20:24:58 #topic Clarify "sanity tests must pass" - specifying `--python` on `ansible-test sanity` call can be problematic 20:25:18 we only say "sanity tests MUST pass" in the requirements, but don't exactly specify what we mean by that 20:26:04 we also say "You MUST run ansible-test sanity from the latest stable ansible-base/ansible-core branch.", and that some sanity errors must not appear 20:27:00 right now some collections use `ansible-test sanity --docker --python xx` with a very short list of python versions, or use it without --docker and with --python also for just a small list of python versions 20:27:37 the problem is a) some sanity tests will not work with all Python versions (many require Python >= 3.6), and b) especially the import and compile tests should be run for (almost) all Python version - and if you specify --python they are only run for that version 20:28:10 Also, some tests, like yamllint, won't work if you don't have a python with libyaml support in your pyyaml install. 20:28:26 one extreme example seems to be ansible.utils, where apparently sanity tests are *only* run with Ansible 2.9 (which already violates the requirements), but also only for one specific Python version 20:28:30 mattclay: yes, good point! 20:28:32 mattclay: Do you think people should ever be using `--python x.y`? 20:28:33 Likewise, shellcheck is skipped if it's not installed. 20:29:05 gundalow: Only to split up their tests into a matrix for parallel testing. 20:29:19 I would personally require that collections must pass `ansible-test sanity --docker` (without any --python), or even require that CI runs that (and if --docker is no option, take special care to make sure that all tests actually run, like tadeboro's collection :) ) 20:29:20 We use --python because we run without --docker. But we do run againts wide range of python versions. 20:29:45 tadeboro: Is that because you can't cache the docker images, so it would be too slow? 20:30:04 Yes, test image on ansible 2.9 is 1.1 GB 20:30:07 felixfontein: That would be my requirement as well 20:30:32 ALl our images combined are 300 MB + are provided by CI provider -> fast as hell ;) 20:30:44 I would suggest something like must run for all python versions that are not specifically excluded as supported in your collection's documentation. Does that address half the problem while not undercutting the other half (yamllint and shellcheck being the other half) 20:31:34 abadger1999: I think it addresses the first half, yes. it still might cause some ignore.txt entries to be missing, so if anyone runs `ansible-test sanity --docker` that could fail 20:32:01 Automation Hub seems to run that, but only for Ansible 2.9. Galaxy doesn't seem to run it 20:32:16 You can also use `--venv` instead of `--docker` for sanity tests, which without `--python` should generally work as well as `--docker`, except for things that can't be pip installed (shellcheck, libyaml, .net core for powershell tests). 20:32:34 Of course that does require you to have all the necessary python versions available. 20:32:46 mattclay: But will probably only use python versions that are available, right? 20:32:46 Which can be a challenge, and is why `--docker` is recommended. 20:33:01 tadeboro: Correct 20:34:09 This is why we run sanity tests in containers using python 2.7, 3.6, 3.7 and 3.8 (ans soon 3.9). This combined with CI caching of pip stuff makes thing go really fast. 20:34:44 if the default container is cached, `ansible-test sanity --docker` is also fast ;) 20:34:58 Also, when using --docker, --not-python 2.6 would also come in handy ;) 20:35:13 we're 10 minutes into "Clarify what 'sanity tests must pass' mean" (and 95min in the meeting) 20:35:30 tadeboro: Eventually there will be support for declaring python versions not to test in a collection, which would have the same effect. 20:35:38 Yeah, negation could be very handy. Especially as new python versions keep coming out. 20:35:59 * acozine must attend to other things 20:36:41 acozine: thanks for your time 20:36:59 the community meeting is always interesting! 20:37:07 :) 20:37:41 suggestion: how about this formulation: "Collections must run an equivalent of `ansible-test sanity --docker`. If they do not use `--docker`, they must make sure that all tests run, in particular the compile and import tests (which should run for all Python versions). Collections can choose to skip certain Python versions that they explicitly do not support; this needs to be documented in 20:37:47 README.md and in every module and plugin (hint: use a docs fragment)." 20:37:57 acozine: thanks for being arond! 20:37:59 +u 20:38:35 felixfontein: +1 20:38:38 When I reviewed collection, I used common sense: single ansible version + single python version is not enough. Missing ansible 2.10 is also no-go. Other is negotiable if the author can explain the reasons. 20:38:39 +1 20:38:57 felixfontein: +1 20:38:59 tadeboro: +1 20:39:50