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

Reply via email to