On Thu, Mar 8, 2018 at 12:22 PM, Jakub Hrozek <jhro...@redhat.com> wrote: > > >> On 8 Mar 2018, at 12:13, Fabiano Fidêncio <fiden...@redhat.com> wrote: >> >> On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek <jhro...@redhat.com> wrote: >>> >>> >>>> On 8 Mar 2018, at 10:33, Fabiano Fidêncio <fiden...@redhat.com> wrote: >>>> >>>> People, >>>> >>>> I've noticed that I'm getting a little bit lost with github and the >>>> way SSSD has its tags organized there. >>>> >>>> As it may actually affect other people (and in case it does not, let's >>>> just skip the following suggestion) ... I'd like to suggest the >>>> following tags to the project: >>>> >>>> - Accepted: We already have it; >>>> >>>> - Rejected: We already have it. >>>> >>>> - Tests needed: This one can either replace the "Changes Requested" >>>> (in case it's split in a few different tags) or be used together ... >>>> but the idea is to identify that tests are missing from a PR without >>>> going through the whole discussions happening there; >>> >>> What do you propose would be the action after tests needed? Should it be a >>> follow up PR, a ticket for the project, a ticket for downstream..? >>> >> >> After the "Tests needed" tag is added the developer should either: >> - Write the tests upstream (considering that we have infra for that, >> which is not the case for all the PRs) > > Here I’m really worried that unless we have a ticket, this won’t happen. Look > at the “CI” milestone in pagure.. So I would say this case should result in > Changes requested, filing a ticket or asking downstream QE to write a test.
Hmm. My original thought is that the PR wouldn't even be pushed without the tests (as it happened already with a few PRs) ... not that we'd push and leave the tag there. > >> - Provide a "link" of the related downstream tests that were >> broken/were added passing >> > > This makes sense, although I would argue this should already be default. But > if you don’t think so, we can try the tag and see how it goes. Hmm. Indeed. I guess we can postpone this at least for now and focus on your "downstream tests passed" tag ... which would be a better investment of time. Agreed? > >> So, summing up, no ticket for the project, no ticket downstream ... >> just making clear that the PR is stalled because "Tests are needed". >> Does that make sense? >> >>> My worry about not supplying tests along with PRs is that the tests will >>> never be supplied..at least not in upstream.. >> >> I understand why you're worried and I agree with that. See the answer >> above and let me know if it fits your expectations. >> >>> >>>> >>>> - Depends on (or something similar): This one can either replace the >>>> "Changes Requested" (in case it's split in a few different tags) or be >>>> used together ... but the idea is to identify that we depend on >>>> somework that still has to be done (either another PR, ticket or >>>> something else that has to be implemented). Mind that I'm not sure >>>> whether we'd be able to simply add a field saying what the PR depends >>>> on … >>> >>> I think this makes sense. At least for a casual observer it would be clear >>> that there is no work needed on this PR. >>> >>>> >>>> - Postponed/Deferred: We have something similar for 2.0, but would be >>>> nice to have a way to clearly see in which release we're going to take >>>> a look into a specific PR without having to dig in the discussions. >>>> Here we could also have 1.16.1, 1.16.2, 2.0, … >>> >>> Tags are cheap, we can even have a postponed/$version. I guess even >>> depends/$PR might be OK as long as we only had a handful of dependecies. >>>> >>>> - Reworked: Although just removing the "Changes Requested" label is >>>> fine, maybe having a tag explicitly saying that something was Reworked >>>> would be a clean way to differentiate between new PRs and PRs which >>>> have been through a review already … >>> >>> I don’t know how this tag would be used, could you give me an example, >>> please? >> >> I usually have no idea (just by a quick look on github) whether a PR >> has been re-worked or it's a new PR that's never been reviewed. >> My impression is that having the "Reworked" tag would make simpler for >> people to jump in and do a follow-up review on what has been addressed >> in the first round(s) of review and then give their ACK instead of >> just leaving it for the reviewer. Of course, the same can be achieved >> without that tag ... so, it's just something that looks more >> "organized" to me. > > OK, if this is something that was hitting you, maybe the tag might make > sense. But, then do you volunteer to maintain these tags? Because since I > didn’t see this as a problem, I’m afraid at least I wouldn’t maintain the > tags. I do volunteer to maintain the tags, for sure. > >> >>> >>>> >>>> Does the suggestion make sense? In case we have an agreement about >>>> this topic, may I re-tag our PRs and start using those new tags from >>>> new PRs? >>> >>> Another tag I was thinking of was “passes downstream tests”. With the >>> amount of time our downstream tests take, I’m not even sure how to >>> integrate them with the usual github flow like travis or centos CI use. So >>> I was thinking about a bot that would nightly scan PRs that have neither >>> “pass” or “fail” tag, bundle those up in an RPM, run the nightly tests and >>> report back using a tag. >> >> I really like the idea! >> >> Another tag that may be added is something like "Urgent" for PRs that >> are *really* *needed* for some specific reason (downstream, release, >> etc …) > > Umm, fine, but how would others find out the list of urgent PRs? Isn’t it > then easier to drop a mail to the list? Hmm. It may be easier to drop a mail to the list or even talk in our weekly phone meeting. So, yes, this one can be dropped. > >> > >>> >>>> >>>> Best Regards, >>>> -- >>>> Fabiano Fidêncio >>>> _______________________________________________ >>>> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org >>>> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org >>> _______________________________________________ >>> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org >>> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org >> >> Let me know in case I was not able to answer all your questions. >> >> Best Regards, >> -- >> Fabiano Fidêncio >> _______________________________________________ >> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org >> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org > _______________________________________________ > sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org > To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org _______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org