Github labels: Suggestion

[Fabiano Fidêncio]

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;

- 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 ...

- 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, ...

- 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 ...

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?

Best Regards,

[Jakub Hrozek]

> On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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..?

My worry about not supplying tests along with PRs is that the tests will never be supplied..at least not in upstream..
> 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?
> 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.
> Best Regards,
>
> Fabiano Fidêncio
> _______________________________________________
> sssd-devel mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
> > On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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)
- Provide a "link" of the related downstream tests that were
broken/were added passing

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.
> > 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 ...)
> > Best Regards,
> >
> > Fabiano Fidêncio
> > _______________________________________________
> > sssd-devel mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
>
> sssd-devel mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

Let me know in case I was not able to answer all your questions.

Best Regards,

[Jakub Hrozek]

> On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
>
> On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
> > > On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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.
> 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.
> 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.
> > > 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?
>
> > > Best Regards,
> > >
> > > Fabiano Fidêncio
> > > _______________________________________________
> > > sssd-devel mailing list -- [e-mail]
> > > To unsubscribe send an email to [e-mail]
> >
> > sssd-devel mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
>
> Let me know in case I was not able to answer all your questions.
> Best Regards,
>
> Fabiano Fidêncio
> _______________________________________________
> sssd-devel mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:22 PM, Jakub Hrozek  wrote:
> > On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
> >
> > On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
> > > > On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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 -- [e-mail]
> > > > To unsubscribe send an email to [e-mail]
> > >
> > > sssd-devel mailing list -- [e-mail]
> > > To unsubscribe send an email to [e-mail]
> >
> > Let me know in case I was not able to answer all your questions.
> > Best Regards,
> >
> > Fabiano Fidêncio
> > _______________________________________________
> > sssd-devel mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
>
> sssd-devel mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Jakub Hrozek]

> On 8 Mar 2018, at 12:30, Fabiano Fidêncio  wrote:
>
> On Thu, Mar 8, 2018 at 12:22 PM, Jakub Hrozek  wrote:
> > > On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
> > >
> > > On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
> > > > > On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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.

So it would mean “the code looks good and will be pushed as long as you sort out tests one way or another” ?
> > > 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?

Well, I don’t think the tests passed tag would come soon (as in, not this week, not the next one). If you see a use-case for more tags, use them. But as Pavel said, I would at least initially add a comment what do you want from the other part of the PR until we find out how the process works best for us.

I mean, I don’t want to impose my workflow on others. If you think it makes sense, let’s try it. Worst thing, we stop using the tags and remove them..
> > > 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 -- [e-mail]
> > > > > To unsubscribe send an email to [e-mail]
> > > >
> > > > sssd-devel mailing list -- [e-mail]
> > > > To unsubscribe send an email to [e-mail]
> > >
> > > Let me know in case I was not able to answer all your questions.
> > > Best Regards,
> > >
> > > Fabiano Fidêncio
> > > _______________________________________________
> > > sssd-devel mailing list -- [e-mail]
> > > To unsubscribe send an email to [e-mail]
> >
> > sssd-devel mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
>
> sssd-devel mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:44 PM, Jakub Hrozek  wrote:
> > On 8 Mar 2018, at 12:30, Fabiano Fidêncio  wrote:
> >
> > On Thu, Mar 8, 2018 at 12:22 PM, Jakub Hrozek  wrote:
> > > > On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
> > > >
> > > > On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
> > > > > > On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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.
>
> So it would mean “the code looks good and will be pushed as long as you sort out tests one way or another” ?

I do think so. But this is something that must be agreed between us (the team).
> > > > 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?
>
> Well, I don’t think the tests passed tag would come soon (as in, not this week, not the next one). If you see a use-case for more tags, use them. But as Pavel said, I would at least initially add a comment what do you want from the other part of the PR until we find out how the process works best for us.
>
> I mean, I don’t want to impose my workflow on others. If you think it makes sense, let’s try it. Worst thing, we stop using the tags and remove them..
> > > > 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 -- [e-mail]
> > > > > > To unsubscribe send an email to [e-mail]
> > > > >
> > > > > sssd-devel mailing list -- [e-mail]
> > > > > To unsubscribe send an email to [e-mail]
> > > >
> > > > Let me know in case I was not able to answer all your questions.
> > > > Best Regards,
> > > >
> > > > Fabiano Fidêncio
> > > > _______________________________________________
> > > > sssd-devel mailing list -- [e-mail]
> > > > To unsubscribe send an email to [e-mail]
> > >
> > > sssd-devel mailing list -- [e-mail]
> > > To unsubscribe send an email to [e-mail]
> >
> > sssd-devel mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
>
> sssd-devel mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Pavel Březina]

On 03/08/2018 12:22 PM, Jakub Hrozek wrote:
> > On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
> >
> > On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
> > > > On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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.

I would use this like this:

a) You push a PR so it can be reviewed and you plan to provide tests later.

b) You push a PR without tests and someone decides tests are mandatory 
for this PR.
> > 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.
> > 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 P >>>
> > > > 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.

Patch dependencies are not that common so we can just set tag 
"depend/blocked" and write PR/ticket it depends on to the PR message as 
we do now.
> > > > 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.

The problem is that github does not sent any notifications that new 
patches were pushed and it does not show in the PR list. So we can use 
this tag to notify reviewers that new patches are pushed, but also 
please always write comment there since I don't usually go through webui 
but only through notification mails.
> > > > 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?

I would not overcomplicate it. Urgent PRs -> irc ping.

[Jakub Hrozek]

> On 8 Mar 2018, at 12:34, Pavel Březina  wrote:
>
> On 03/08/2018 12:22 PM, Jakub Hrozek wrote:
> > > On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
> > >
> > > On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
> > > > > On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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.
>
> I would use this like this:
>
> a) You push a PR so it can be reviewed and you plan to provide tests later.
>
> b) You push a PR without tests and someone decides tests are mandatory for this PR.
> > > 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.
> > > 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 P >>>
> > > > > 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.
>
> Patch dependencies are not that common so we can just set tag "depend/blocked" and write PR/ticket it depends on to the PR message as we do now.
> > > > > 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.
>
> The problem is that github does not sent any notifications that new patches were pushed and it does not show in the PR list. So we can use this tag to notify reviewers that new patches are pushed, but also please always write comment there since I don't usually go through webui but only through notification mails.

You get the “synchronized” notification on sssd-devel. I’m not sure if you do directly from github. My personal workflow was to check the PR dashboard for anything without a tag and consider that as needing review.

But again, I don’t want to impose my workflow for others, if others think this tag makes sense, let’s try using it.

(btw what about the github workflow document? Should we amend it after some time of using the new tags?)
> > > > > 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?
>
> I would not overcomplicate it. Urgent PRs -> irc ping.
> _______________________________________________
> sssd-devel mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Pavel Březina]

On 03/08/2018 12:46 PM, Jakub Hrozek wrote:
> > On 8 Mar 2018, at 12:34, Pavel Březina  wrote:
> >
> > On 03/08/2018 12:22 PM, Jakub Hrozek wrote:
> > > > On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
> > > >
> > > > On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
> > > > > > On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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.
> >
> > I would use this like this:
> >
> > a) You push a PR so it can be reviewed and you plan to provide tests later.
> >
> > b) You push a PR without tests and someone decides tests are mandatory for this PR.
> > > > 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.
> > > > 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 P >>>
> > > > > > 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.
> >
> > Patch dependencies are not that common so we can just set tag "depend/blocked" and write PR/ticket it depends on to the PR message as we do now.
> > > > > > 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.
> >
> > The problem is that github does not sent any notifications that new patches were pushed and it does not show in the PR list. So we can use this tag to notify reviewers that new patches are pushed, but also please always write comment there since I don't usually go through webui but only through notification mails.
>
> You get the “synchronized” notification on sssd-devel. I’m not sure if you do directly from github. My personal workflow was to check the PR dashboard for anything without a tag and consider that as needing review.

I actually have filter that deletes PR messages that goes to sssd-devel. 
I get personal notification from github. But I may revisit those.
> But again, I don’t want to impose my workflow for others, if others think this tag makes sense, let’s try using it.
>
> (btw what about the github workflow document? Should we amend it after some time of using the new tags?)

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?
> >
> > I would not overcomplicate it. Urgent PRs -> irc ping.
> > _______________________________________________
> > sssd-devel mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
>
> sssd-devel mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:46 PM, Jakub Hrozek  wrote:
> > On 8 Mar 2018, at 12:34, Pavel Březina  wrote:
> >
> > On 03/08/2018 12:22 PM, Jakub Hrozek wrote:
> > > > On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:
> > > >
> > > > On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
> > > > > > On 8 Mar 2018, at 10:33, Fabiano Fidêncio  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.
> >
> > I would use this like this:
> >
> > a) You push a PR so it can be reviewed and you plan to provide tests later.
> >
> > b) You push a PR without tests and someone decides tests are mandatory for this PR.
> > > > 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.
> > > > 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 P >>>
> > > > > > 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.
> >
> > Patch dependencies are not that common so we can just set tag "depend/blocked" and write PR/ticket it depends on to the PR message as we do now.
> > > > > > 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.
> >
> > The problem is that github does not sent any notifications that new patches were pushed and it does not show in the PR list. So we can use this tag to notify reviewers that new patches are pushed, but also please always write comment there since I don't usually go through webui but only through notification mails.
>
> You get the “synchronized” notification on sssd-devel. I’m not sure if you do directly from github. My personal workflow was to check the PR dashboard for anything without a tag and consider that as needing review.
>
> But again, I don’t want to impose my workflow for others, if others think this tag makes sense, let’s try using it.
>
> (btw what about the github workflow document? Should we amend it after some time of using the new tags?)

Yep, that would be the way to go.
> > > > > > 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?
> >
> > I would not overcomplicate it. Urgent PRs -> irc ping.
> > _______________________________________________
> > sssd-devel mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
>
> sssd-devel mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]