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

Defining the proposals / GIP process #1

Closed
akien-mga opened this issue Apr 10, 2019 · 16 comments
Closed

Defining the proposals / GIP process #1

akien-mga opened this issue Apr 10, 2019 · 16 comments
Labels

Comments

@akien-mga
Copy link
Member

akien-mga commented Apr 10, 2019

During the last GodotCon in Brussels, core developers discussed the idea to move feature proposals outside the main godotengine/godot repository to let the latter focus on bug reports and current developments.

This new godot-proposals repository would be the main place for users to propose new features for the engine, or any kind of big change that requires discussion and consensus on design, implementation, scope, etc.

The exact workflow needs to be defined, which is the purpose of this first issue.

Proposed workflow

The current proposal discussed at GodotCon and refined on IRC is the following:

  • Issues are used for all proposals. One or more issue templates will be created to ensure that proposals include the necessary information, in particular detailed use cases. All users are welcome to use reactions to express their support of any proposal, and to discuss details.

  • Once a proposal has been sufficiently discussed, a decision should be taken, to either:

    • Reject it if the proposal is not wanted
    • Approve it and turn it into a formal Godot Improvement Proposal (GIP)

GIPs are text documents (probably in Markdown or plain text) following a standard template (to be defined in its own GIP) which summarize an approved proposal. Those text documents serve as reference for the implementation and can be linked to in the roadmap for upcoming Godot versions.

GIPs are submitted via Pull Requests following a process yet to be defined. Once a GIP is merged, implementation can start and the feature is formally approved for merging in the engine, provided that it's done following the GIP.


Answers to some likely questions:

Why do we do that?

As the community grows, managing the issue tracker of the main godotengine/godot repository is increasingly difficult. Users come with proposals every day, and while many are good, triaging and discussing them takes a lot of focus away from actual bug reports, which should have the priority. Labels help, but not as much as we'd like. The godot repository currently has close to 900 feature proposals (many very old and not getting any attention) and 1300 enhancement issues (most of those would stay there, but some with a bigger scope would also be candidate for a GIP).

Moving things around doesn't automatically solve this issue, but it allows us a better separation of concerns, and to use features differently. Having a dedicated repo for proposals means that we can have a proposals-specific workflow, with its own labels and milestones. We can also bring new users to the issue triage team that would focus on managing proposals, which should be more interesting for many than handling bug reports and complex issues on the main repository.

There might be some degree of wishful thinking, but with good guidelines and a well defined workflow, we could definitely see a lot of improvement. At any rate, it's worth a shot.

Beyond the handling of end user proposals, we also have a need for a formal RFC (Request For Comments) system for more involved proposals from core contributors, so we want to try both at the same time using this hybrid informal issue / formal PR workflow.

What about feature proposals currently in the main repo, or future proposals made in the wrong location?

GitHub recently introduced a feature to move issues between repositories, so we can use that to move all feature proposals from godotengine/godot to godot-proposals. The same can be done for any new proposal made in godotengine/godot, or for bug reports mistakenly reported here.

We will first define, refine and test the GIP workflow before moving anything from godotengine/godot to this repository.


More to come probably as you ask questions and refine the proposed workflow.

@akien-mga
Copy link
Member Author

Note: For this issue, let's focus on the general workflow outlined above and refine it. Further details, like the issue template(s) for proposals, the workflow for going from an issue to a formal GIP, the exact template to use for GIPs, etc. can be discussed in other issues, once they have been validated as necessary in the general workflow.

@karroffel
Copy link

Awesome! I really like the idea of going through a formal process first before doing significant changes!

It seems the focus of this first draft is the urgent processing of "informal" issues from the main-repository. I think this is a good idea to have more separation of "wishes" and "problems".

I do feel like the more important part is the "formal" step of planning big changes via RFCs/GIPs.
Finding a good workflow for formal proposals is probably getting more important as time goes on and suggestions are posted as issues here.

What does it take to get a GIP accepted? When does it count as rejected? What is the planned "timeline"/"lifecycle" for GIPs?

I think the Rust RFC process had quite some time to be refined into something decent.
(at this point Rust RFCs are getting really big and hard to keep track of, so they are looking into solutions that scale even more, but I think beginning with a Github-discussion based RFC model is a step in a good direction)


My "high-level" proposal for the GIP process

Any significant changes to Godot (new Nodes, changes to scripting interface, animation system etc) have to be formalized in a GIP first. Godot is growing in popularity and giving the users a space to take part in designing new features could bring a stronger sense of "group-ownership", as well as less surprises! With many people jumping in on feature designs more viewpoints can be considered and maybe alternative solutions can be found.

Pre-GIP

Issues are the "gateway" into GIPs, so people post suggestions and ideas and everyone can freely discuss those.
What lifts an issue into a GIP is a person actually implementing the GIP. There are no specific restrictions on what can be made into a GIP. Since the GIP process is more formal (but not too formal that nobody wants to deal with it), only ideas that spark enough interest would actually motivate somebody to put in the effort of preparing a GIP.

GIP process

GIPs should be stated as if the feature/change is already implemented, explaining why this change "was made" and what the consequences of it are.

A GIP is assigned to a "team". Teams are contributors/"advisors" which take the final decision to accept or close a GIP. All members of a team have to agree to the change, each team has to consist of 3 people or more.

There are teams for semantic categories of the engine. A "core team" which is responsible for the very base of the code. Basically all changes that would affect the core/ or build-system are to be approved by the core team. An "editor/UX" team would be involved in most user-facing and editor changes. Etc etc. Teams can grow and team members can agree to introduce new people into teams.

Since approval cannot be done by a single person, a bot would be the only entity with push rights and all members of the team have to acknowledge a GIP for it to be merged.

When a GIP comes to an end, a "final commenting period" could be announced, which would give a final "visibility push" to the GIP and potentially new people could bring in new insights that might spark new discussions. This might not be needed and I think observing how this plays out in practice could decide if it's needed/useful or not.

Post-GIP

A GIP should be a guideline for implementing the proposed changes. Hence, after a GIP is merged, a tracking issue is added in the main repository to track the implementation status.

Prototypes could be created while a GIP is still being discussed but no code related to the GIP will be merged into the main repository.


This process should somewhat closely mirror the Rust one. I like the general structure of it, since the place where design discussions happen is clearly defined and the main repository would be mostly for implementation discussions.

@reduz
Copy link
Member

reduz commented Apr 11, 2019

@karroffel That's too formal for a project of this nature in my opinion (and open source in general), where everyone works on whatever they want when they want.

I think this place is great to discuss features, but nothing warrants that they will be implemented.
That said, as @akien-mga described, it's great that they reach a state where there is agreement that this feature is wanted (and we of course should have tags noting this). This would encourage contributors to take ideas from here and work on them. Currently, the feature proposal issues in the main repo are a sea of chaos.

Of course, there should be core contributors (with more knowledge of the internals and engine usage) needed to give final approval to features. Otherwise I can see a bunch of users agreeing to something that us core devs may not be interested in, or that we may think it's a bad idea, or not even practically possible.

@sdfgeoff
Copy link

That's too formal for a project of this nature in my opinion (and open source in general), where everyone works on whatever they want when they want.

I think when you have a project of this size receiving hundreds of issues and a couple dozen pull requests, you need to formalized this process. I think that's what this whole issue about. Maybe @karroffel's suggestion is a bit too formal, but in order to improve the stability and long term quality of Godot, some formalization does need to be introduced. The fact that there are now 400PR's open when last year there were only 200 at a time shows that something needs to change.

So, because we are an open source project, we should perhaps look at how successful open source project manage their issues/proposals/PR's. Some projects that I would consider good as case studies include:

  • Blender
  • Linux Kernel
  • Tensorflow (one of the most starred projects on Github)

These are well established projects, and both support thousands of people going about their daily lives. Godot currently doesn't support as many, so solutions for them may not be 100% applicable, but is is probably still worth looking at them.

What Akien Proposes

    +-----------------+       +----------------+
    |                 |       | formal Godot   |
    | Open Issue on   +----->---Improval       |
    | godot-proposals |       | Proposal (GIP) |
    |                 |       | PR on GIP repo |
    +-----------------+       +----------------+
                                       |
                                       v
    +-----------------+       +----------------+
    |                 |       | Development    |
    | Code review     <---------of Code        |
    | (I suppose)     |       |                |
    +-----------------+       +----------------+
            |
            v
    +-----------------+
    |                 |
    |  Merge to Godot |
    |                 |
    +-----------------+

What Blender does
As far as I can tell from here: https://wiki.blender.org/wiki/Process/Contributing_Code
They seem to have much what Godot currently has:

  1. People write patches
  2. Other people review them, both feature wise and technical
  3. If it passes review, it gets merged

I think this works for Blender because the primary users are Artists, who don't suddenly start opening PR's on Blender. As a result, the number of users is much much higher than the number of developers. With Godot, most users have some understanding of software, and are much more likely to submit a patch or code a features. Also, the use of a mailing list probably decreases the number of people opening and then abandoning PR's

I also think that Blender providing such a rich plugin/extension API means that many things that would normally appear as a PR instead appear as addons. Providing more power to Godot's plugin system may allow rejected patches to appear as addons, or for current engine features to be moved into addons.

It's worth noting from Blender that:

  1. The review process can be slow.
  2. Patches instead of PR's
  3. The module owners are listed (https://wiki.blender.org/wiki/Process/Module_Owners/List) to make them easier to find/contact
  4. No work happens directly on Master

Linux Kernel
See https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html

The process here is fairly similar to blender: patches via mailing list with review by module/code owners before final approval by Linus.

TensorFlow
https://github.com/tensorflow/tensorflow/blob/master/CONTRIBUTING.md

  1. PR's against main github repo
  2. Approval and review by a "tensorflow team member"
  3. Submitted to an internal tensorflow repository
  4. Exposed on public tensorflow github

Another thing of note is that:

Full new features (e.g., a new op implementing a cutting-edge algorithm) typically will live in tensorflow/addons to get some airtime before decision is made regarding whether they are to be migrated to the core.


Interesting things I notice:

  1. All of them have separate internal repositories: the public can see the repository, but cannot directly open PR's that are one-click-away-from-merge on them. The final review process and merge is internal to the organization supporting the project. This means that every single merge has to go via a patchfile or a multiple remotes or some other process. This extra step may add some "security" against small patches being merged without sufficient review.
  2. Mailing lists + patches are a thing.
  3. Some sort of staging. Tensorflow has the addons repository, blender has graphicall. This allows users to test the changes with a certain patch without having to become developer themselves.
  4. Way less development on master. No development happens on master.

@StraToN
Copy link
Member

StraToN commented Apr 11, 2019

@reduz I don't think @karroffel 's proposal avoids anyone working on what she/he wants. Indeed the described process is very formal, but it is required for anyone searching for a Feature Request to start working on, that said Feature Request has been "greenlit".
I think the question @karroffel tried to address is: who grants this "green light"?

@QbieShay
Copy link

I think something needs to be clarified as well: what happens to features that core developers want to implement? Do they go through GIP too?
What I could observe so far is that user-proposed features go through much more review (both from other users and engine developers) than the features proposed within the core team.

My own opinion on this is that every new feature, from when the GIP process is started, should pass through GIP, for the following reasons:

  1. Users use Godot a lot more than the engine developers, so, even if the final decision on what to do belongs to the core team, users should be involved in this process as much as possible, providing different point of view, use cases, feedback, and so on.
  2. With the size that Godot is reaching, it's very easy that some people work on the same feature without even knowing it. Having a form of discussion and agreement respects the time of the contributors ( since most of the contributors do this in their free time)
  3. Having a slower process for the addition of feature will give a bit more of visibility to bugs and, while one is waiting for a feature to be approved/rejected, there is a chance that they will take a look at bugs instead.
  4. Godot has reached a size where nobody really has a complete view on the entire engine anymore, which is why giving contributors and engine developer more time to review features and the implementation of those is, if not a very good idea, necessary.

Review and decision process

I believe that having small teams dedicated to certain parts of the engine is a good idea, as long as they all share the same vision.
For this reason, guidelines of the engine development should be made clearer.
Having more clear guideline of the engine's vision and philosophy will also enable the users to understand better why a feature was denied / implemented in a different way/ and so on while maintaining user's engagement in the development process.
(I imagine closing feature requests with "this does not comply to the engine guidelines, see ")

Ultimately, the decision about accepting a feature request or not will belong to the core team assigned to the feature request. Every other core team is welcome to give their own opinions and feedback, but the decision makers should be the ones that have the most experience with a certain part of the engine (since they are supposedly more experienced and knowledgeable regarding that part of the engine).

@bojidar-bg
Copy link

I would like to chip in with my 2¢ proposal on how GIPs should be handled:

  1. Users submit an issue on this repo, containing some usecases and suggesting a new feature to handle them.
  2. The issue is discussed on by whomever is interested. More usecases and suggestions are added, though nothing is final yet.
  3. At some point, someone decides to do the work, and opens a PR on this repo containing a formalized GIP. The PR is set to auto-close the discussion issue when merged.
  4. Some discussion continues on the PR; people unhappy with the proposed solution are free to open a competing PR (potentially reusing the name of the GIP file, in order to prevent both PRs from getting merged).
  5. Core developers look over submitted GIPs and Approve/Reject them. Similar to how PRs on the main repo are handled, there is no required amount of people approving, but rejections require changes or argumentation. Potentially, "requested reviews" are required before merging, in order to ensure that requested teams have been given a chance to look over the proposal.
  6. The PR is merged or closed. If merged, a tracking issue containing a link to the GIP is opened on the main repository. That issue can then be assigned, claimed, etc.
  7. Afterwards, things continue as normal. The final code might be slightly different from the GIP, in case there were unforeseen problems with the proposal.

In this scheme there are a few possibilities for shortcuts or skips:

  • A proposal PR can be created without a prior discussion issue, if deemed straightforward enough. Big proposals without a prior issue will need compelling usecases listed before getting merged.
  • A code PR can be opened on the main repo without a prior GIP. In case of controversies, a GIP discussion issue might be opened, thus going through all the normal steps of a proposal.

@StraToN
Copy link
Member

StraToN commented Apr 18, 2019

What happens if a contributor issues a PR on godotengine/godot with a new feature that was not validated by a GIP process beforehand?

@fire
Copy link
Member

fire commented Apr 29, 2019

I recommend we split any proposal that is separable. We have a few proposals here that is a large list of issues and some of them could be extracted.

This allows the proposals to be individually voted on and given the attention they deserve.

@akien-mga
Copy link
Member Author

Proposed workflow, take 2

Reading through this, it seems we more or less have a consensus on what the workflow should be:

  • Users and contributors open issues in this repository to propose changes
  • These proposals get discussed and eventually approved or rejected
    • Who can approve/reject proposals is to be further defined, but it would likely be teams of core contributors competent on the proposals' topic
  • Proposals for substantial changes have to go through the GIP process.
    • "Substantial" is the key here. Consensual proposals which don't require a substantial amount of code/design discussion can go from the "approved issue" to a PR on the code repository directly. Similarly, as outlined by @bojidar-bg, contributors can still open PRs directly on the code repository without going the GIP process beforehand, but if the proposed PR is not consensual, they might be asked to go through the GIP process. This approach should address @reduz's concerns (which I share) about adding too much bureaucracy to any contribution. We can adjust the cursor regarding what requires a formal GIP as we go based on our experience.
  • The GIP process implies writing a text file following a given template and adding it to this repository via a PR. The template should include a summary of the proposal, motivation/use cases and implementation details. The PR can be used to further refine the proposal's implementation details, and once merged the original proposal issue is closed. An issue should be open on the main repository to keep track of the GIP's implementation.
  • Whether a proposal issue needs to be turned into a GIP is decided by the team responsible of the proposal's area, or can be prompted by a user/contributor who decided to submit a PR with a GIP for the currently discussed issue.

Labels

We should have labels for each part of the issue's and PR's lifecycle.

For issues, I suggest:

  1. New issues get either of these labels:
    • proposal: for all new issues while they are in the discussion phase. (We could also go with "no label" for this state as all new issues would be proposals by default, but I personally like my issues all labelled... this is up for bikeshedding.)
    • meta: for issues like this one which refer to the GIP process/this repo itself and are not proposals for engine features. The next two stages are not relevant for such issues.
    • archived: our usual catch-it-all for any invalid issue (duplicate, not a proposal, not Godot-related, etc.). The next two stages are not relevant for such issues.
  2. in-review: once a discussion has reached a state where stakeholders can take a decision, this label is set by issue triagers or relevant team members. The modalities of review have to be determined and might be team-dependent, as long as they are documented and the decision process transparent. (Alternatively, we could skip this label and consider that any proposal is "in review" from the beginning. We would thus need a different system to filter the proposals which are ready for reaching a decision, e.g. adding a review-request for the team. But IMO labels are easier to filter with in GitHub's current state to quickly get a team's TODO list.)
  3. Final stage is either of those:
    • approved: the team approves the feature and considers it can go straight to implementation (not a substantial change requiring more formal steps).
    • gip-ready: the team pre-approves the feature and wants it to be formalized into a GIP (PR). Within a given timeframe, a contributor should be assigned to the issue to write the GIP. The issue is closed as fixed once the matching GIP PR is merged.
    • rejected: the team rejects the feature. If new elements come up later on that challenge this decision, team members or issue triagers can decide to reopen the issue and go back to in-review stage. Can be set directly without going through in-review, if the team considers that a proposal is not worth discussion further.

Style-wise, we may want to prefix those with status: or similar so that all lifecycle labels are grouped together when assigning labels, similar to topic: and platform: labels that we have on the code repo (and that we might want to include in this GIP repo too).

For PRs, I'm not sure yet, it would likely depend on the GIP template and what kind of requirements are needed for a GIP to be merge-ready. It might be that review-requests and reviews/PR approvals might be sufficient and that we don't need further status labels for PRs.


Further discussion

@sdfgeoff: The module owners are listed (https://wiki.blender.org/wiki/Process/Module_Owners/List) to make them easier to find/contact

We should have something similar. We have Teams in the GitHub Organization, but sadly those are made private if you're not organization member, so we need to duplicate this information in another public location.

We also need to create more teams and formalize their members (many contributors today have an area of expertise but no dedicated team they are a member of).

@QbieShay: I think something needs to be clarified as well: what happens to features that core developers want to implement? Do they go through GIP too?
What I could observe so far is that user-proposed features go through much more review (both from other users and engine developers) than the features proposed within the core team.

That's a good question and probably warrants further discussion. As per what @bojidar-bg and I wrote above, I would still allow contributors to open PRs directly without making a prior GIP, but reviews may require a formal GIP to be written if the contribution needs more discussion on the design.

All core contributors will be strongly encouraged to go through the GIP workflow, and I do believe that most will do given how requested it is from core contributors.

Some big projects from core contributors might be difficult to formalize as GIP before writing the code though, e.g. @reduz is now figuring out Vulkan and how to design it all as he goes. A prior GIP could have been written for adding the Vulkan support and some implementation ideas, but the final code will likely be quite different from what could have been documented before doing the work.

That being said, even an imperfect GIP that mostly documents the core contributors' general approval of a feature like "Implement Vulkan support" is probably worth having.

@QbieShay: I believe that having small teams dedicated to certain parts of the engine is a good idea, as long as they all share the same vision.
For this reason, guidelines of the engine development should be made clearer.
Having more clear guideline of the engine's vision and philosophy will also enable the users to understand better why a feature was denied / implemented in a different way/ and so on while maintaining user's engagement in the development process.

Definitely, we need more written guidelines on the development philosophy and what kind of contributions are desired or not. This would greatly simplify our work when rejecting PRs, as currently it can take a while to put the words on why a given PR "feels" wrong even if it implements a potentially desired feature in a potentially "correct" way.


Next steps

To move on with the process, I plan to:

  • Formalize the issues workflow and document it in the README.md. For now I will mention the GIP (PR) workflow but leave out the details. I will open a dedicated issue to discuss the GIP template.
  • Create the labels necessary for the issues lifecycle.
  • Add an issue template for proposals (once the GIP workflow is defined, we can also add a template for PRs).
  • Formalize and document the teams and their members.
  • Start applying the process on existing issues in this repository and migrate some from godotengine/godot.
  • Once the workflow seems good, we will mass transfer all feature proposals from godotengine/godot, and document everywhere that new feature proposals should come to this repo.

@bojidar-bg
Copy link

Just another cent:
Do we want GIPs for other parts of the Godot project, which are not kept track of in the main repository? For example, do we want GIPs for the documentation ("written guidelines" as per above)? What about GIPs for the asset-library or website? And finally, what about GIP-s related to the blender exporters and other misc. repositories we have in Godot?

@sdfgeoff
Copy link

sdfgeoff commented May 23, 2019

Some big projects from core contributors might be difficult to formalize as GIP before writing the code though, e.g. @reduz is now figuring out Vulkan and how to design it all as he goes. A prior GIP could have been written for adding the Vulkan support and some implementation ideas, but the final code will likely be quite different from what could have been documented before doing the work.

So small changes don't need GIP's, medium ones do, and super-huge ones don't again? In my mind, this kind-of defeats the purpose.Implement before design sounds like a recipe for unclear architecture and later refactors. I think it is better to /require/ a GIP. If somone feels they cannot design one, they can do prototypes outside of Godot, do research, do whatever it needs for them to be able to design a solution.
The aim of the GIP is to ensure that design happens before implementation. We should take care not to break this.

In fact, I would be in favor of having some sort of fixed metric a bot can detect in order to enforce having GIP's. For example: every PR on godotengine/godot will not be able to merge if it has more than 2000 lines of change/deltas or changes more than 4 files unless there is an associated GIP. This is something that can be applied fairly and consistently, and make it obvious if a GIP is required. Exactly where this bar is set, and if there are exceptions will need to be discovered but having some sort of metric and adhering to it will be extremely useful. I suspect even large trivial changes, such as changing the name of a #define would probably be sane having a GIP anyway.

Having some sort of metrics on the current state of Godot merges will allow us to figure out if the GIP is useful or not. For example, we could use reverts as a measure of "mistakes" (currently 0.2% of commits are reverted). We could then compare before/after GIP and see if GIP is serving it's intended purpose. If we don't have things we can measure, we can't know if GIP is being useful or just adding needless bureaucracy.

As far as I can tell, some aims that can be metric'd may be:

  • managing the issue tracker better (fewer issues? fewer old issues?)
  • reduce time of MR's in review -> accepted (75% of current MR's are inactive)
  • reduce number of reworks of an MR before inclusion
  • reduce number of closed/abandoned MR's (16% of MR's are not merged)

Scraping these metrics sounds like a bit of fun, so if someone wants to build a list.....

@QbieShay
Copy link

QbieShay commented May 23, 2019

@akien-mga

Some big projects from core contributors might be difficult to formalize as GIP before writing the code though, e.g. @reduz is now figuring out Vulkan and how to design it all as he goes. A prior GIP could have been written for adding the Vulkan support and some implementation ideas, but the final code will likely be quite different from what could have been documented before doing the work.

I don't agree with this. Huge architectural changes should require discussion, especially if it's something new. GIP is not only about the code, it's also about architecture design and taking into considerations different needs that might not be clear from the beginning. Also, whoever will implement the proposal will have the chance to be exposed to more sources, different point of views. Maybe nothing will change of their initial idea, maybe it will.

I don't see how giving time/space to discuss certain big changes in a public space is bad/unadvisable.

GIP doesn't necessairly mean agreeing on implementantion, it might also mean that different opinions on the same topic get a common place where they can be discussed, reviewed, and so on.

@sdfgeoff
Copy link

What is the status of GIP? I'd like to see this (or a similar) workflow be implemented

@reduz
Copy link
Member

reduz commented Sep 3, 2019

I took what was discussed here together with discussions online on irc and put together a new proposal for a system that aims to solve what we know for certain that are problems we are having.

@aaronfranke
Copy link
Member

I'm closing this issue as solved, as the proposal / GIP process has been well defined by a combination of this issue, #10, the PRs to this repo, and many of the other meta proposals, plus those linked above (and in the future, below) this comment. This issue has served its purpose, and further discussion on the proposal process should be done in other meta issues and PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants