Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New labelling system for Help Wanted tickets #59

Closed
jasonblais opened this issue Feb 2, 2019 · 18 comments
Closed

New labelling system for Help Wanted tickets #59

jasonblais opened this issue Feb 2, 2019 · 18 comments
Labels
Contributor Journey Process

Comments

@jasonblais
Copy link
Owner

jasonblais commented Feb 2, 2019 •
edited
Loading

Below is the proposed labelling system for Help Wanted tickets.

Key changes to existing process

1. Ticket status:

  • Help Wanted: Recommended by GitHub so that new contributors can more easily find tickets to help with https://help.github.com/articles/helping-new-contributors-find-your-project-with-labels/
  • Up for Grabs: Signifies whether ticket is still available or taken by a community member.
    • Automate this via slash commands, e.g. /taken
  • PR Submitted: Signifies when a contribution is made.
    • Automate based on keywords, e.g. when GH ticket is linked in the help wanted ticket

2. Difficulty:

3. Repository:

Mattermost has a lot of repositories, but all help wanted tickets are created in the mattermost-server repo. Thus, it's important to be clear which repository changes are submitted to (see an example where contributor was confused and submitted their changes to the mattermost-redux repo instead of mattermost-webapp: mattermost/mattermost-redux#761)

  • Repository/mattermost-server
  • Repository/mattermost-webapp
  • Repository/mattermost-redux
  • Repository/mattermost-mobile
  • Repository/mattermost-plugin-jira
  • etc..

4. Language:

Contributors can then filter based on their skill sets, or find easy tickets for a language they want to learn

  • Language/Go
  • Language/JavaScript

5. (Optional) Framework:

Contributors interesting in a specific framework such as React Native or Redux can filter for these tickets

  • Framework/Redux
  • Framework/React Native
  • Framework/ReactJS

6. (Optional) Area:

Which area the feature is related to. This is helpful for larger campaigns contributors are interested in. Below are some examples, some of which are already in use (e.g. Add E2E Tests

  • Area/APIv4
  • Area/Add E2E Tests
  • Area/Plugins
  • Area/Build
  • Area/Dev Tools
  • Area/Code Quality
  • ...

7. When PR submitted:
@jasonblais
Copy link
Owner Author

/cc @hanzei

@jasonblais
Copy link
Owner Author

jasonblais commented Feb 2, 2019 •
edited
Loading

I do wonder, for the first section, if Help Wanted and PR Submitted labels are sufficient. First indicates we're looking for help, second that a PR was submitted. Is Up for Grabs needed in that case?

@hanzei
Copy link

hanzei commented Feb 6, 2019

Yes Up for Grabs is needed IMO. I see the process as:

  • Create a HW ticke and add Help Wanted and Up for Grabs.
  • A community member wants to work on this: Remove Up for Grabs
  • The PR is submitted: Add PR Submitted

@hanzei
Copy link

hanzei commented Feb 6, 2019

Are the labels listed at 6. just examples?

👍 on the rest.

On tought on 7. My goal is to reduce to the time a PR is "in flight". Sometimes a PR gets in a state where every reviewer approved the PR but it didn't get merged like in mattermost/mattermost#10198. To avoid this, my proposal is:

  1. Remove 3: Ready to Merge
  2. When all reviews are done, remove 2: Dev Review.
  3. If a PR should not be merged jet, apply one of two labels:
  • Awaiting Next Release if today is feature complete (Maybe we should rename the label to make it more clear)
  • Awaiting Next Release if this dependents another PR.
  • Are there other reasons to not merge a PR?

Site note: What do you think about moving all labels from the pattern $TOPIC: $STATE to $TOPIC/$State., e.g. Docs/Doneinstead ofDocs: Done`.

@jasonblais
Copy link
Owner Author

1 - Makes sense regarding 'up for grabs'. That won't add too much process overhead?

2 - Yea, labels listed in 6 are examples.

3 - About PRs waiting to be merged

  • Awaiting Next Release - this is typically only for a day, 0/5 if needed? We can, but we also have the milestone that signifies which version it should be merged to
  • We have Awaiting PR on repos, when there's a PR blocking reviews or merge. We could use that one

4 -

What do you think about moving all labels from the pattern $TOPIC: $STATE to $TOPIC/$State., e.g. Docs/Doneinstead ofDocs: Done`.

My preference is consistency so I'm all for it.

@jasonblais
Copy link
Owner Author

Thanks for the feedback by the way! And adjusted wording for 6 to clarify the labels are examples.

@hanzei
Copy link

hanzei commented Feb 6, 2019

  1. We have a three step progress. With only one label this isn't possible to represent. The overhead is not so much. Maybe we can automate PR Submitted at one point.

And: If PR Submitted doesn't help, we can remove it.

  1. 👍

  2. I'm not sure I understand you feedback. Do you agree with my proposal to remove 3: Ready to Merge'? Awaiting Next Release` is only used one day, but still useful IMO.

  3. 👍

@jasonblais
Copy link
Owner Author

jasonblais commented Feb 7, 2019 •
edited
Loading

1 - We are moving from one label (Up for Grabs) to three, which means it'll likely be up to you and I to keep the ticket labels up-to-date. Because other team members may not remember them.

That's the only downside. I'm 0/5, so we can try it and see how it goes.

3 - We are adding a new process that would affect the whole R&D team (20-30 people), so I want to make sure it's needed. There's a risk where people will forget (it took some time to get everyone up to speed with the CherryPickXXX tickets).

Is there an example PR where our current labels failed?

For the example PR you shared above (mattermost/mattermost#10198), would the change in labels have helped? Or was merging it simply forgotten/missed?

Also, the PR is marked as CherryPickApproved --> does it need to be cherry-picked for 5.8? mattermost/mattermost#10198 I don't think that's been done yet.

4 - By the way, I forgot to mention - we should prepare a list of the changes and let people know so they can bring up concerns if any, since those labels are used in various R&D workflows. There may also be some documentation to update.

But again, 👍 for consistency, I don't think the team would have major concerns.

@jasonblais
Copy link
Owner Author

@hanzei Are you okay if I present the proposal for help wanted ticket process, as described in the issue?

I think the two outstanding topics are 3 + 4, neither which affects help wanted issues.

@hanzei
Copy link

hanzei commented Feb 11, 2019

About mattermost/mattermost#10198:
In https://community-daily.mattermost.com/core/pl/cccaafmjwtn98f8dxxzx98u31y it was decided to get this into v5.8.0. I just followed this decision.

Back to the original topic:

  1. Help Wanted can be added to every new HW ticket, hence we can add it easily to our current automatisation.
    If we forget to add PR Submitted, it's not a big deal. BTW adding/removing this labels is something I would consider PR coaches to in the future.

  2. Let defer this for later.

  3. Maybe you can discuss this as well in your meeting, since it's just a minor change? 0/5, I don't mind if we also defer this.

@jasonblais
Copy link
Owner Author

+1 on all of the above, and I can mention #4 as well 👍

@jasonblais
Copy link
Owner Author

jasonblais commented Mar 2, 2019 •
edited
Loading

@hanzei What are your thoughts on the following proposal.

It is similar to what we discussed before, but removes labels for repositories. Instead, I've added a checklist for help wanted ticket creator to confirm they've applied all appropriate labels and listed the relevant Mattermost repos in issue description.

A - Labelling system for all GitHub issues

Many GitHub repositories have adopted $TOPIC/$STATE for their GitHub labels. Propose we adopt the same for consistency.

The adoption shouldn't cause major changes - as an example, Docs: Done would become Docs/Done, CherryPickApproved would become CherryPick/Approved. It has also been incorporated in the Help Wanted ticket proposal above.

The one exception is 1: PM Review, 2: Dev Review, 3: Review Complete, which are intentionally marked to appear at the top of label list, because they are used most often. We'll want to keep this order unchanged.

B - Labelling system for Help Wanted tickets

Key changes to existing process

1. Ticket status:

  • Help Wanted: Recommended by GitHub so that new contributors can more easily find tickets to help with https://help.github.com/articles/helping-new-contributors-find-your-project-with-labels/
  • Up for Grabs: Signifies whether ticket is still available or taken by a community member.
    • Automate this via slash commands, e.g. /taken
  • PR Submitted: Signifies when a contribution is made.
    • Automate based on keywords, e.g. when GH ticket is linked in the help wanted ticket

2. Difficulty:

3. Language:

Contributors can then filter based on their skill sets, or find easy tickets for a language they want to learn

  • Language/Go
  • Language/JavaScript

4. (Optional) Framework:

Contributors interesting in a specific framework such as React Native or Redux can filter for these tickets

  • Framework/Redux
  • Framework/React Native
  • Framework/ReactJS
  • Framework/Cypress
  • ...

5. (Optional) Area:

Which area the feature is related to. This is helpful for larger campaigns contributors are interested in. Below are some examples, some of which are already in use (e.g. Add E2E Tests

  • Area/APIv4
  • Area/Add E2E Tests
  • Area/Plugins
  • Area/Build
  • Area/Dev Tools
  • Area/Code Quality
  • ...

6. When PR submitted:

C - Help Wanted issue template

When a JIRA ticket gets applied a Help Wanted fix version, a Help Wanted issue is created in GitHub.

We use a template that automatically adds an introduction for each GitHub issue, so that contributors have all the resources they need to get started.

Propose we add an internal checklist at the end of the issue to confirm the Help Wanted issue contains all other relevant information, including

  • required labels for difficulty and language
  • optional labels for framework and area, when applicable
  • list of repositories to submit changes to mentioned in issue description
If you're interested please comment here and come [join our "Contributors" community channel](https://community.mattermost.com/core/channels/tickets) on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please  [join our "Developers" community channel](https://community.mattermost.com/core/channels/developers).

New contributors please see our [Developer's Guide](https://developers.mattermost.com/contribute/getting-started/).

----

**Notes**: [Jira ticket](https://mattermost.atlassian.net/browse/{{issue_key}})


{{issue_description}}

----

**Help Wanted issue creator checklist**

This checklist is for the Mattermost core committer who created the Help Wanted issue. Please confirm you have completed the following:

- [ ] Listed all repositories to submit changes in the issue description (e.g. mattermost-redux, mattermost-mobile and mattermost-webapp)
- [ ] Added labels for difficulty and language
- [ ] Added `first good issue` label, if applicable
- [ ] Added labels for framework and area, if applicable

@hanzei
Copy link

hanzei commented Mar 4, 2019

This is awesome. I like the solution of having a checklist. I have no objections.

Thanks for creating this proposal and (hopefully) driving this rework to completion.

@saturninoabril
Copy link

For the "(Optional) Framework", can we add "Framework/Cypress"?

Do we need "QA Review" somewhere "When PR submitted" or it will be indirectly part of PM Review?

@jasonblais
Copy link
Owner Author

Thanks Saturn!

  1. Added "Framework/Cypress" to the list, and added a "..." to indicate there may be other frameworks in future
  2. Good point, I added a note that "QA Review" may be included as part of the PM Review stage.

@hanzei
Copy link

hanzei commented Mar 25, 2019 •
edited
Loading

I've run in the situation a couple of times, where a PR gets merged because even though a awaited PR isn't merged.

What do you think about

  • Renaming Awaiting Awaiting PR -> Do not merge/Awaiting PR
  • Renaming Awaiting Next Release -> Do not merge/Awaiting Next Release
  • Renaming DO NOT MERGE -> Do not Merge, because caps lock is a bit to much

@jasonblais
Copy link
Owner Author

Seems fine to me 👍

@lieut-data lieut-data mentioned this issue Apr 15, 2019
Merged
@jasonblais
Copy link
Owner Author

jasonblais commented May 3, 2019 •
edited
Loading

This is now complete, closing.

Edit: The third part (HW issue template) wasn't implemented yet, so crated a separate issue for that #60

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Contributor Journey Process
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@saturninoabril @jasonblais @hanzei