search

dwyl / dwylbot

:robot: Automating our GitHub Workflow to improve team communication/collaboration and reduce tedious repetition!
28 stars 7 forks source link

Feature: warning on in-progress issues with no time estimate #61

Closed ghost closed 7 years ago

ghost commented 7 years ago

When an issue has the "in-progress" label and no estimation time label has been added then add a comment with "Please estimate the issue before starting working on it" and assign the author of the issue

SimonLab commented 7 years ago

I think the author of the issue should be assigned only if there are no assignee. Otherwise we should keep the current assignee.

ghost commented 7 years ago

@SimonLab thinking about this further I think it only needs to comment - it should @username the person who applied the in-progress label (as opposed to the author of the issue) as this person should be that person thanks to the in-progress functionality

SimonLab commented 7 years ago

I think we need to add the name of the assignee on the comment if it's define, if not then the author of the issue. For example the author if this issue is @markwilliamfirth but I was assigned to it and it was my responsibility to add the time estimation

ghost commented 7 years ago

So because we have https://github.com/dwyl/dwylbot/issues/7 working, it means that if the in-progress label is attached, there must (by this logic) be someone assigned - so maybe it makes sense to @username the person that is assigned?

SimonLab commented 7 years ago

The rule in #7 (the issue must have an assignee with the in-progress label) can happen at the same time as this rule so an assignee might not be defined. I think adding the author of the issue on the comment or the assignee if there are defined should work

ghost commented 7 years ago

@SimonLab maybe the missing time estimate should only be triggered after someone has been assigned and in progress label has been added? i.e. allow #7 to function first and then this applies after that has happened? Otherwise the issue author might been unnecessarily assigned, for example when the execution of the task has nothing to do with the author

SimonLab commented 7 years ago

The user who adds the in-progress label is the one who will be mentioned on the issue. This user should be logically the one who is assigned to the issue.

ghost commented 7 years ago

@SimonLab yep but assigning of the user who adds the inprogress label should already occur in #7 here: https://github.com/dwyl/dwylbot/issues/7#issuecomment-308164125

So a missing time estimate should only be picked up by dwylbot on three conditions:

  1. if someone is assigned
  2. the inprogress label is attached
  3. there is no time estimate

If all of these are true then dwylbot should remove the in-progress label, comment and @username the user to say that a time estimate needs to happen before the issue can be in-progress

dwylbot[bot] commented 7 years ago

@iteles the in-progress label has been added to this issue without a time estimation. Please add a time estimation to this issue before applying the in-progress label.

iteles commented 7 years ago

My two cents: Firstly, I would suggest that we don't create dependencies between rules as ultimately we want to have a system where people can switch rules on and off; dependencies make for a more complicated UX and require more complex logic. As such, we want to cover our bases.

Secondly, I think there is a separate issue which has now been mentioned both here and in https://github.com/dwyl/dwylbot/issues/7#issuecomment-308164125 which is assigning of the person who added the in-progress label to the issue. This seems like it's still under some discussion so let's decouple it for now into a separate issue, creating more flexibility in the system by having it as a separate rule.

In light of this, we need to consider what happens if there is no times estimate on an in-progress label and no assignee, which as I understand is the subject of this issue.

ghost commented 7 years ago

@iteles

I would suggest that we don't create dependencies between rules

I don't think this is a problem, as the flow should be assign > time estimate > in-progress, so there should probably be a dependency here. I thought the idea of dwylbot is to ensure the dwyl process is followed, or is the point to allow people to customise all rules?

This seems like it's still under some discussion

@SimonLab and I actually discussed this at our last catch up and agreed to do this

iteles commented 7 years ago

Cool, if we keep it as a separate and additional rule, we satisfy both the criteria of getting it done and ensuring the system is flexible for future use with the tiniest marginal cost, I think it's worth it 👍

The point is to ensure dwyl processes but remember that these are also under constant testing and improvement so if flexibility is a very small marginal cost (under an hour I'm thinking in this case), I would suggest it's worth it.

ghost commented 7 years ago

@iteles if there are two separate rules independent of each other then dwylbot may post twice (unnecessarily) for both issues. If there is a dependency, dwylbot will only post at each step in the process that isn't followed

iteles commented 7 years ago

@SimonLab Mark raises a good point above! This could get annoying quickly and if people start ignoring dwylbot, that's a more serious problem than lack of flexibility.

SimonLab commented 7 years ago

I'll try to explain how I see the flow of this rule based on the "rule template" describe in the issue #83 and I think we should try to formalise the rule this way to avoid confusion, so at the moment I can see two versions of the rule, each of them has a different trigger event:

version 1: The rule in-progress_no-estimation is triggered by the event: in-progress label is added to the issue. If the list of labels doesn't contain a time estimation label Then apply on the issue the following actions:

version 2: The rule in-progress_no-estimation is triggered by the event: an assignee is added to the issue. If the list of labels doesn't contain a time estimation label AND the list of labels has the "in-progress" label Then apply on the issue the following actions:

I think the version 1 is the one I prefer, it follows the logic that if in-progress is added a time estimation need to be define too. It's true that if there are no assignee there will be also a comment from the other rule "in-progress no-assignee" but it doesn't bother me as the rules are different. But if we want to combine multiple error rules in one comment, we need to create a new issue for that and think how to implement it. For example we need to define what does it means having multiple errors at the same time, should we check for all the errors in a certain time laps? I don't think it is a trivial feature to implement, at least it need more thinking.

The version 2 follow, I think, the flow you described @markwilliamfirth, is this right? This means that we only check this rule if there is an assignee and the in-progress label has already been added. This feel a bit strange for me as we should check for the time estimation before or at the same time the in-progress label is added.

if we implement the version 2 we will also have the following flow for example

So we can see that the user will receive also two comments at the same time. So to conclude I still prefer the version 1.

@iteles @markwilliamfirth Let me know if this comment is not clear. I try to described my thought process but I might I missed something. Let me know also if you find another solution or another version on how this rule could work, thanks :+1:

ghost commented 7 years ago

@SimonLab

If we want to enforce the dwyl flow, then it should flow as:

  1. assignee
  2. time estimate
  3. in-progress

here the in-progress label is still on the issue (is this still true?). the user add an assignee -> comment to ask the user to add a time estimation

In this case the in-progress label should be removed. To be clear this is how I see it working:

Case 1 The rule in-progress_no-assignee is triggered by: in progress label is added If there is no assignee Then these actions occur: in progress label is removed add an @username comment on the issue user who added in progress label is assigned

Case 2 The rule in-progress_no-estimation is triggered by: in progress label is added If the list of labels doesn't contain a time estimation if there is an assignee Then these actions occur: in progress label is removed add an @username comment on the issue user who added in progress label is assigned (if they aren't already)

This encourages a step flow, so that the user understands the order of the steps that they need to follow. Yes in Case 1 they have also violated not having a time estimate, but I don't think it is useful to comment twice in the same step. But splitting out the comments it will help dwylers remember the step by step flow and what order they should do things in.

iteles commented 7 years ago

@SimonLab @markwilliamfirth I foresee removing the in-progress label would be problematic for scrum master and client use cases. Is there a way to do this without removing the in-progress label?

Let me explain: the point of the in-progress label is so that everyone involved in the project can see at a glance what is being worked on. As a scrum master I do this multiple times a day across projects. I've also sat next to POs who do this every morning as a quick check on the prioritisation of the project.

This means that it's substantially more important to me to have an in-progress label than to have an assignee. If someone hasn't followed process and assigned themselves, as a backup I can still go in and see who applied the label; but if the label is removed (it's possible that the person will get to work straight away and not notice this has been done for hours or days), then I lose visibility of what's happening in the project.

Does that make sense?

SimonLab commented 7 years ago

@markwilliamfirth your process will indeed work if dwylbot remove the "in-progress" label. Like Ines said above I'm not sure about dwylbot forcing the user to add again the "in-progress" label. On my part I would feel a bit annoyed to reapply the label and repeating the steps twice (for no assignee and no time estimate). I would still go with my version 1 of my previous comment (https://github.com/dwyl/dwylbot/issues/61#issuecomment-309011447) and try to find a solution to combine multiple errors in one message. I let you decide what you think is best for this issue @markwilliamfirth & @iteles A/B testing?

iteles commented 7 years ago

@markwilliamfirth Pick a starting point and let's start testing with that, but the above is a real concern of mine and this is my most used label in the filters 👍

ghost commented 7 years ago

I think the issue here is that the plan and primary focus of dwylbot is currently ambiguous.

I am of the understanding that the primary purpose of dwylbot is to force dwylers to adhere to the dwyl development flow and teach them to use the proper flow. The reasoning being that this ultimately saves the organisation time and money. dwylbot also has many other secondary benefits and the long-term vision means that other organisations can adopt the dwyl flow or perhaps create their own rules, but as I understand this is the primary purpose of the app: https://github.com/dwyl/dwylbot/blame/master/README.md#L47 https://github.com/dwyl/dwylbot/blame/master/README.md#L64

@iteles please can you clarify if the purpose of the application is to force people to adhere to and learn the flow, or as your comments suggest, encourage it but allow people not to follow it if they so wish?

If someone hasn't followed process and assigned themselves, as a backup I can still go in and see who applied the label

If it is always true that the person who attaches the in-progress label is the person who should be assigned, then alternatively we can automate assigning that person upon them adding the in-progress label. This removes the problem. I don't think that this is always the case though, so I don't know how much of a useful backup this is to warrant affecting this decision. If the following occurs both you and the dwyler will be notified by dwylbot and so it will be clear what has happened and the dwyler should learn from the mistake.

If the in-progress label being removed really is a big issue (perhaps as you suggest because it might not be noticed by the dwyler) then perhaps a process-error label could be applied instead or there may be an alternative solution. If the time delay is removed it will mean it will be very unlikely that dwylbots reaction will not be noticed by the dwyler.

On my part I would feel a bit annoy to reapply the label and repeating the steps twice (for no assignee and no time estimate).

@SimonLab this is intentional - it reminds the user of the correct flow at each step. Therefore the flow is never ambiguous and the dwyler learns quickly to avoid receiving comments, because indeed they are annoying.

iteles commented 7 years ago

@iteles please can you clarify if the purpose of the application is to force people to adhere to and learn the flow, or as your comments suggest, encourage it but allow people not to follow it if they so wish?

Definitely to help people learn the flow and adhere to it. (agree, this needed to be more clearly stated by me earlier on).

My note was just that it would be more detrimental to remove the label than to not have an assignee. Maybe assigning the person who adds the in-progress label also enforces process as people learn not to add in-progress labels to other people's issues (this is super annoying when it happens).

Just airing a concern as I use this label to guide me on a daily basis, preventing me from having to tap anyone on the shoulder to ask what's going on. Happy to test whatever we land on.

SimonLab commented 7 years ago

@markwilliamfirth agree that the first version of dwylbot focus on how to teach dwyl contribution guideline to the team. However I think we should be careful to not force people. My last comment was trying to describe this feeling, if I'm forced to repeat multiple time the same steps I will surely uninstall dwylbot from my repos. Hoewever if I receive warning messages but not force to add again labels I think I will be more inclined to follow the guideline as the rules are not too intrusive in my workflow. But maybe I'm wrong and if we force people everything will be ok, need to test.

I'm still not sure if there is a final decision for the rule:

ghost commented 7 years ago

@SimonLab bearing all of the above in mind, I think we should make the following changes:

Case 1 The rule in-progress_no-assignee is triggered by: in progress label is added If there is no assignee after 30 seconds Then these actions occur: user who added in progress label is assigned dwylbot comments saying this person added the in-progress label without an assignee, so they were assigned

Case 2 The rule in-progress_no-estimation is triggered by: in progress label is added If the list of labels doesn't contain a time estimation after 30 seconds Then these actions occur: add a workflow-error label add an @username comment on the issue asking for a time estimation

SimonLab commented 7 years ago

@markwilliamfirth I think this issue is ready to test. At the moment dwylbot will add a comment for each cases you described above. The other issues you've described will done on separate issue as there is quiet a bit of work to do on them:

If we want to combine the rules and just have one comment, this can be done with #92 (combine comments) and https://github.com/dwyl/dwylbot/issues/129#issuecomment-311026688 (idea how to combine rules)

Adding the "workflow-error" labels needs to be applied on every errors and needs also to be removed when the errors is resolved. This need a bit more thought on how to implement this, see #107

dwylbot[bot] commented 7 years ago

@markwilliamfirth the in-progress label has been added to this issue without an Assignee. dwylbot has automatically assigned you.

Any questions, complaints, feedback, contributions? Discuss If you prefer, you can also send us anonymous feedback: https://dwyl-feedback.herokuapp.com/feedback/new

dwylbot[bot] commented 7 years ago

@markwilliamfirth the in-progress label has been added to this issue without a time estimation. Please add a time estimation to this issue before applying the in-progress label.

Any questions, complaints, feedback, contributions? Discuss If you prefer, you can also send us anonymous feedback: https://dwyl-feedback.herokuapp.com/feedback/new

dwylbot[bot] commented 7 years ago

@markwilliamfirth the in-progress label has been added to this issue without an Assignee. dwylbot has automatically assigned you.

Any questions, complaints, feedback, contributions? Discuss If you prefer, you can also send us anonymous feedback: https://dwyl-feedback.herokuapp.com/feedback/new

dwylbot[bot] commented 7 years ago

@markwilliamfirth the in-progress label has been added to this issue without a time estimation. Please add a time estimation to this issue before applying the in-progress label.

Any questions, complaints, feedback, contributions? Discuss If you prefer, you can also send us anonymous feedback: https://dwyl-feedback.herokuapp.com/feedback/new

ghost commented 7 years ago

Yes @SimonLab this works 👍