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.

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

Update Bug triage guidelines #10683

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tetrapod00
Copy link
Contributor

@tetrapod00 tetrapod00 commented Feb 13, 2025

Work in progress.

  • Update the list of labels to match the current list.
  • List which GitHub features we use in a more up to date way
  • Clarify when archived is used
  • Use sphinx headers instead of bold :)
  • Rewrote entirely the milestones description. While it was true in palces, it's wrong enough that it deserved a rewrite from first principles.

@tetrapod00 tetrapod00 added enhancement area:contributing Issues and PRs related to the Contributing/Development section of the documentation labels Feb 13, 2025
Godot engine GitHub organization, and even then not in all cases. Any contributor
is welcome to take on any issue, after discussing it with other contributors.
If you would like to pick up an issue (and no one else is working on it), all
you have to do is leave a comment in the issue.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could write some additional content about the general workflow here.

the next minor version, or to ``4.x``. As a rule, we assign new features to
``4.x`` version initially to avoid continually re-triaging a PR from version to
version. However, a PR being in ``4.x`` does not mean it won't be merged; it's
just the default for new features.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph needs the most attention to set expectations.


Labels
~~~~~~

The following `labels <https://github.com/godotengine/godot/labels>`__ are
currently defined in the Godot repository:

**Categories:**
Categories:
^^^^^^^^^^^
Copy link
Contributor Author

@tetrapod00 tetrapod00 Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through the whole list, and I consider the current descriptions to be acceptable, complete, and correct to my knowledge.

But I'm sure we could improve these further if we wanted to?

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a great start! Will do a more in-depth review of this soon but it looks good

@tetrapod00 tetrapod00 force-pushed the bug-triage-update branch 3 times, most recently from fe81f76 to 4cf2b65 Compare February 15, 2025 21:25
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally this looks great, will do another pass soon and see what information might be missing from the sections expanded on currently

Comment on lines 128 to 135
- *2D*: relates to 2D-specific issues. Should be coupled with one of the labels
below, and should not be coupled with *3D*.
- *3D*: relates to 3D-specific issues. Should be coupled with one of the labels
below, and should not be coupled with *2D*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two are supposed to be related to 2D and 3D nodes specifically

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I've seen it's used frequently along with topic:rendering, including about features that aren't actually nodes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but what I'm describing is the intended use

Godot engine GitHub organization, and even then not in all cases. Any contributor
is welcome to take on any issue, after discussing it with other contributors.
If you would like to pick up an issue (and no one else is working on it), all
you have to do is leave a comment in the issue.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
you have to do is leave a comment in the issue.
you have to do is leave a comment on the issue.

Also we might want to clarify here that there are issues that aren't necessarily open for general contributions, but should always be handled by maintainers, so might want to not state explicitly that anyone is welcome to take on any issue at least

Comment on lines 259 to 264
All pull requests are assigned to a milestone. By default, enhancement and
feature PRs are assigned to the ``4.x`` milestone, and bugs are assigned to the
current minor version milestone, such as ``4.5``. Towards the end of a minor
version's development, PRs currently in the minor version milestone are
reassigned to either the major version milestone (``4.x``), or the next minor
version milestone (such as ``4.6``).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section should make it clear that PRs are reassigned only if they are not considered for the version, as this makes it sound like all PRs are moved before the version is released

Comment on lines 266 to 267
When a pull request is approved and merged, it is moved from ``4.x`` to the
milestone for the current minor engine version, such as ``4.5``.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens before it is merged, when the review process is complete and the production team decides it is ready to go

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tricky to word well, since:

  • We don't always follow the prescriptive guide for what to put in 4.4 vs 4.x, or when to move PRs
  • The production team "approval" is somewhat opaque
  • Being in 4.4 and having multiple relevant approvals still doesnt always guarantee "merge soon"

So I'm not sure whether to write the prescriptive ideal case or to hedge a bit to account for some inconsistency in usage of milestones.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That aside we normally don't merge PRs that's on a .x milestone, either this part should be removed entirely or it should say that a PR is moved to a specific milestone when it's considered ready to merge, but regardless the specific order of events here doesn't reflect our process

@tetrapod00 tetrapod00 force-pushed the bug-triage-update branch 2 times, most recently from 6b0b807 to 9657032 Compare February 17, 2025 19:31
@tetrapod00
Copy link
Contributor Author

Addressed the review, but left conversations unresolved since a lot of this is subjective

@AThousandShips
Copy link
Member

I think this looks good now, will do a pass later today and see what areas might be missing

@tetrapod00 tetrapod00 marked this pull request as ready for review February 20, 2025 04:25
@AThousandShips
Copy link
Member

Sorry for the delay things came up, will give this a final pass today and I think it should be a good start to improving things

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the exception of a few notes I think this is ready, this is a major improvement and any areas we might still be missing can be improved on in future PRs

All pull requests are assigned to a milestone. By default, enhancement and
feature PRs are assigned to the ``4.x`` milestone, and bugs are assigned to the
current minor version milestone, such as ``4.5``. Towards the end of the minor
version's development, PRs currently in the ``4.5`` milestone are reassessed. If
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version's development, PRs currently in the ``4.5`` milestone are reassessed. If
version's development, PRs currently in that milestone are reassessed. If

To avoid confusion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I had it like this before anyway, but thought it might be clearer with a specific 4.5 example. I changed it back

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me!

Thanks so much for updating this, I've kept telling people that this page was probably outdated for years without actually taking the time to update it... I see now that there had already been significant updates, and this just rounds it up quite well!

As far as possible, we try to assign labels (and milestones, when relevant)
to both issues and pull requests.
We only use the assignees feature for Godot maintainers who are members of the
Godot engine GitHub organization, and even then not in all cases. For other
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Godot engine GitHub organization, and even then not in all cases. For other
Godot Engine GitHub organization, and even then not in all cases. For other


- *Archived*: either a duplicate of another issue, or invalid. Such an
issue would also be closed.
- *Archived*: used to filter closed issues.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- *Archived*: used to filter closed issues.
- *Archived*: used to filter issues closed with a resolution other than "fixed".

`godot-proposals <https://github.com/godotengine/godot-proposals>`__ instead.
PRs which add new features but do not have a corresponding proposal use this
label.
- *Feature proposal*: Used for PRs adding new features which do not have a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with other items:

Suggested change
- *Feature proposal*: Used for PRs adding new features which do not have a
- *Feature proposal*: used for PRs adding new features which do not have a

Comment on lines +87 to +88
- *Feature proposal*: Used for PRs adding new features which do not have a
corresponding proposal use this label. The label is removed when a feature
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to have been a shift in understanding of this label, this wasn't what it meant originally.

The description of enhancement mentions that it's for changes to existing functionality, so feature proposal should cover the addition of new features - whether or not they have a linked proposal.

It should probably be renamed to new feature, the name dates back to when we had feature proposals as issues in the main tracker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I updated it recently to match the observed usage. Personally I do think it would be good to distinguish between new features and enhancements, as originally intended and as you're proposing here.

But this sounds like a case where we should change how the label is applied in practice before changing the docs for it?

organization members
- Each issue and pull request (PR) is categorized with a set of *labels*,
sometimes called "tags".
- Each issue and PR is assigned to a *milestone*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Each issue and PR is assigned to a *milestone*.
- Each PR is assigned to a *milestone*. Some issues can also be assigned to a *milestone* (see below).

compatibility-breaking changes that will be considered for Godot 5.0, in many
years.

Issues are assigned to a milestone, such as ``4.5``, if they are related to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Issues are assigned to a milestone, such as ``4.5``, if they are related to
Issues are assigned to current development milestone, such as ``4.5``, if they are related to


Issues are assigned to a milestone, such as ``4.5``, if they are related to
features introduced in that engine version, or are bugs (regressions) in that
version. Additionally, all issues completed during that engine version are added
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version. Additionally, all issues completed during that engine version are added
version. Additionally, all issues completed during the development of that engine version are added


All pull requests are assigned to a milestone. By default, enhancement and
feature PRs are assigned to the ``4.x`` milestone, and bugs are assigned to the
current minor version milestone, such as ``4.5``. Towards the end of the minor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
current minor version milestone, such as ``4.5``. Towards the end of the minor
current development milestone, such as ``4.5``. Towards the end of the minor

Issues are assigned to a milestone, such as ``4.5``, if they are related to
features introduced in that engine version, or are bugs (regressions) in that
version. Additionally, all issues completed during that engine version are added
to the milestone. We don't always use the ``4.x`` milestone for issues, since by
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
to the milestone. We don't always use the ``4.x`` milestone for issues, since by
to the milestone, so that users can see at a glance in which minor version an issue was first fixed.
We don't always use the ``4.x`` milestone for issues, since by

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:contributing Issues and PRs related to the Contributing/Development section of the documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants