Bors RFCs

The Draft RFCs are over there. This is just an archive of accepted RFCs.

What's an RFC?

Major changes and events are usually proposed by opening an RFC. What counts as "major" can get subjective, but any of these will count:

Feature RFC’s, which include:
- All breaking changes, such as removing or renaming commands, bors.toml configuration options, environment configuration options, changing the name or packaging of the OTP application (which would break people’s deployment scripts), or bumping the Erlang or Elixir version requirement (we will probably make an RFC putting breaking dependency bumps onto some kind of schedule)
- Adding new features with config options and commands, because they need to be tested and maintained to avoid breakage when users rely on them
Informational RFC’s, which include:
- Starting a talk, meetup, or social networking account that will be expected to officially “represent bors-ng”
- Documenting design issues, deciding to never implement a feature, proposing an experiment, or recording a proof-generated insight
Process RFC’s, which include:
- Changing the RFC process, the organization of the issue tracker or the support forum, or other community infrastructure
- Amending the Code of Conduct

Major changes do not include:

Most moderation actions
Bumping a hex.pm dependency, since the Distillery release process automatically pulls those in anyway
Refactoring the codebase
Making a blog post
Fixing something that is clearly a bug, such as breaking the Not Rocket Science Rule, crashing, or displaying wrong information
Minor look and feel improvements, or even minor feature additions to the dashboard that aren’t likely to translate to a we-will-not-break-this commitment

If you submit a pull request to implement a new feature without going through the RFC process, it may be closed with a polite request to submit an RFC first.

Once you've made sure it's appropriate for an RFC, open a new topic in the Draft RFCs section of the forum. Fill out the included template, tag it as feature, process, or informational, and then respond to any feedback.

RFCs rarely go through the process unchanged. The goal is to refine your proposal so that it can answer most sensible objections on its own, without having to reread the entire discussion thread. "Out-of-band" discussions, either in forum private messages, company chatrooms, or even in-person meetings, are common; just make sure you post a summary on the in-band discussion thread, and that anything truly important gets included in the RFC text itself. As the discussion goes along, try to keep the RFC text up-to-date with whatever consensus seems to emerge.

At some point, a member of the project leadership will place the RFC into Final Comment Period, the FCP RFCs category. After two final weeks of remaining open, your RFC will finally be accepted or rejected. The FCP is usually pretty quiet, since it's only really opened after a consensus has already emerged. Once it's approved, your RFC will be numbered and archived.

After an RFC has been accepted

Once an RFC becomes "approved," authors may implement it and submit the feature as a pull request to the bors-ng repo. Being approved is not a rubber stamp, and in particular still does not mean the feature will ultimately be merged; it does mean that in principle all the major stakeholders have agreed to the feature and are amenable to merging it.

Furthermore, the fact that a given RFC has been accepted implies nothing about what priority is assigned to its implementation, nor does it imply anything about whether a developer has been assigned the task of implementing the feature. While it is not necessary that the author of the RFC also write the implementation, it is by far the most effective way to see an RFC through to completion: authors should not expect that other project developers will take on responsibility for implementing their accepted feature.

If an accepted RFC needs to be modified, please flag it for a moderator, using the "Something Else" option and mentioning what needs changed. Most of the time, an RFC won't need changed after it's been accepted, but the nature of this process means that we cannot expect every accepted RFC to actually reflect what the end result will be once the change has actually been tested out. Only very minor changes should be submitted as amendments. More substantial changes should be new RFCs, with a note added to the original RFC.

Recorded information and announcements.

Changes to the development or RFC process.

Summary: This RFC proposes to expand, and make more explicit, decision-making for bors-ng. It puts in place a template for new proposals (with typical sections such as Motivation, Summary, Drawbacks), a plan for accepting and rejecting them, and sets the stage for better governance changes in the future (it doesn't set up a core team, but once the core team is put in place, it'll follow the RFC process). This "initial RFC" shall be approved following its own process; nobody seemed to have a problem with it during the spitballing stage.

Motivation

Some of you might remember way back when @khodzha opened a pull request for an E-easy issue, but @Macarse, who had never even realized the issue existed, wasn’t sure if it was a good idea or not. I (@notriddle) overruled their concerns, mostly because I figured it would be kind of mean to have a newbie open a pull request just to have it rejected with “actually, we didn’t want that anyway”. This essentially indicates a mismatch of expectations: @khodzha and @notriddle thought that the decision had already been made, while @Macarse still thought it was up for debate.

This is not the only time I ended up making judgement without very much discussion in response to an issue report, pull request, or support thread. Even when there is feedback, it isn’t very organized, and the design decisions that are made there get mixed with support questions, bug reports, and announcements. We need a statement of record about what has been decided and who decided it, and it needs to strike the reasonable balance between “set in stone” and “overridden on the Benevolent Dictator Pro Tempore’s daily whims”. It also needs to maintain the balance between “maintaining a coherent features set and overall vision” and “getting community involvement in decision making”.

Guide-level explanation

What's an RFC?

Major changes and events are usually proposed by opening an RFC. What counts as "major" can get subjective, but any of these will count:

Feature RFC’s, which include:
- All breaking changes, such as removing or renaming commands, bors.toml configuration options, environment configuration options, changing the name or packaging of the OTP application (which would break people’s deployment scripts), or bumping the Erlang or Elixir version requirement (we will probably make an RFC putting breaking dependency bumps onto some kind of schedule)
- Adding new features with config options and commands, because they need to be tested and maintained to avoid breakage when users rely on them
Informational RFC’s, which include:
- Starting a talk, meetup, or social networking account that will be expected to officially “represent bors-ng”
- Documenting design issues, deciding to never implement a feature, proposing an experiment, or recording a proof-generated insight
Process RFC’s, which include:
- Changing the RFC process, the organization of the issue tracker or the support forum, or other community infrastructure
- Amending the Code of Conduct

Major changes do not include:

Most moderation actions
Bumping a hex.pm dependency, since the Distillery release process automatically pulls those in anyway
Refactoring the codebase
Making a blog post
Fixing something that is clearly a bug, such as breaking the Not Rocket Science Rule, crashing, or displaying wrong information
Minor look and feel improvements, or even minor feature additions to the dashboard that aren’t likely to translate to a we-will-not-break-this commitment

If you submit a pull request to implement a new feature without going through the RFC process, it may be closed with a polite request to submit an RFC first.

How to submit an RFC

Once you've made sure it's appropriate for an RFC, open a new topic in the #development:draft-rfcs section of the forum. Fill out the included template, tag it as feature, process, or informational, and then respond to any feedback.

At some point, a member of the project leadership will place the RFC into Final Comment Period, the #development:fcp-rfcs category. After two final weeks of remaining open, your RFC will finally be accepted or rejected. The FCP is usually pretty quiet, since it's only really opened after a consensus has already emerged. Once it's approved, your RFC will be numbered and archived.

After an RFC has been accepted

Reference-level explanation

The project leadership

Throughout this RFC, the project leadership will be a role given a number of responsibilities. In particular, project leadership approves and rejects RFC’s.

Currently, the project leadership is Michael Howell, also known as @notriddle, the Benevolent Dictator Pro Tempore. Future RFC’s are expected to replace him with a team structure, who will act in consensus as the project leadership.

The RFC process

There are four RFC categories, each of which corresponds to a stage:

#development:draft-rfcs : This is the first actual stage in the process. The responsibilities of all participants are spelled out below this list.
- #development:fcp-rfcs : The last part of the discussion stage, where the discussion is timeboxed at two weeks. An RFC will always go through this phase before being accepted, and will likely, though not necessarily, go through this phase before being rejected.
#development:approved-rfcs : An RFC that has been accepted for implementation.
#development:rejected-rfcs : An RFC that has not been accepted, and likely will not be accepted.
#development:postponed-rfcs : An RFC in this category will likely be moved back to draft later. This phase is used when no process seems to be made, yet the leadership doesn't want to accept or reject it yet either, such as if the discussion is blocked on something external to the project.

Responsibilities of the author

The individual (or, I guess, an organization would be allowed to act as an author as long as they can agree on everything out-of-band before acting together as a "hive mind") who opens an RFC up will act as the RFC’s “author”. They are responsible for defending or editing the RFC based on criticism that comes up during the discussion, acting as the RFC’s advocate.

If the original author no longer wishes to champion the RFC, they can hand it over to someone else (obviously, the new author and the old author both have to publicly agree to this) or withdraw the RFC. If the RFC is withdrawn, it will be closed.

For any discussions that happened before the RFC was opened that the author thinks are relevant, the author should copy any knowledge they created into the RFC or the discussion thread. They should also copy any necessary knowledge from the RFC’s own discussion thread into the RFC, so that the RFC’s text can stand alone. Linking to external discussions is allowed, but they should copy or summarize it in case the third-party discussion host goes away.

Responsibilities of the project leadership

While anyone can participate in the RFC discussion, the final decision about approving or rejecting an RFC lies with the project leadership (and the mod team, if they decide that an “RFC” is pure spam they can just delete it, but the mod team can’t just accept an RFC). They do not have to put it up to a community vote, though they will need to come to an internal consensus, and they may ask for a non-binding poll if they think it would help.

The project leadership is expected to voice their concerns, allowing the author to edit or defend the RFC. As described in the Code of Conduct, the project leadership shall make every effort to maintain a professional atmosphere, and to make the author feel that they are being helped, not hindered. The win condition is one where everyone involved feels that they made the best possible decision with what they have.

Project leadership must put an RFC into a “Final Comment Period” if they wish to accept it, and may (but need not) do so if they wish to reject it. Final Comment Period lasts two weeks (14 days), to make sure that anyone who receives RFC’s in weekly digests has a chance to respond. After FCP, all non-staff commenting is disabled and project leadership must make their final decision, moving the RFC from the discussion phase on to the accepted or rejected category.

The RFC document must also be de-wiki-fied after the RFC is closed, and if the RFC is accepted, it must be assigned a number, archived as a GitHub Page, and have a tracking issue created.

Responsibilities for all discussion participants

The most important rule is to keep tabs on the discussion, and avoid relitigating a point that has already been made. As always, follow the Code of Conduct.

It is highly recommended that third-party participants avoid acting as the devil’s advocate. Instead, please represent what your actually needs are in an honest way; the personal experience of individuals who are not the champion or a member of the project is the most valuable contribution that third-party participants can make.

Also, participants should send any mechanical corrections (spelling, grammar, formatting) to the author as a Private Message, not as a public post in the thread. The project leadership appreciates mechanical fixes, and the author probably does too, but once they’ve been fixed any post that corrects them is just clutter, not something that people who want to understand what happened should have to read through years later. Alternatively, if the participant has a high enough permission level on the forum, they may simply perform mechanical fixes to the RFC themselves.

Meta-discussion, surrounding the quality of the discussion itself rather than the RFC text directly, should be kept in a separate #meta thread or in a Private Message. And while the ethical implications of an RFC are perfectly welcome in the in-band discussion thread, problems that are related to the background or behavior of participants should, again, be kept to the #meta section, to PMs, or should simply be handled by flagging the offending post and moving on.

Drawbacks

So far, this RFC basically copies what Rust does, with a couple of tweaks to fit Bors’s slower development pace and a few notes and bits pulled from Swift Evolution, BEP, and PEP.

This means we’re already very familiar with the major drawbacks:

Public RFC’s have been known to attract a lot of attention, sometimes creating a discussion that is so long that it’s impossible to read it all, which compounds the problem because it causes people to post the same concern twice.
Public RFC’s also attract concern trolls, since they have so much attention paid to them, making the length problem even worse.
Settling on a single canonical discussion medium requires one to make final decisions about a bunch of trade-offs, such as ease of archive (the contents of a git repository excel at this one), moderation tools (Discourse excels at this one), familiarity (GitHub Issues excel at this one), ease of entry (chat rooms excel at this one), sophisticated review features (reviewable.io and Gerrit excel at this one), and others that I can’t think of right off the bat. The disorganized status quo allows participants to make an in-the-moment best call.
- Since this version of the RFC RFC uses Discourse, it sacrifices ease of entry and archive in favor of Discourse's customizability and moderation tools.
This RFC process is essentially waterfall; implementing the feature is expected to happen after the feature has been planned. While amending an RFC is expected to happen on occasion, it's heavy-weight enough, and this process is slow enough, that an implementer may choose to stick to the plan even if they know it's wrong to avoid having to propose a change of plan.
It is also very easy to spend a lot of time arguing about one minor problem, only to find out later that there's something way more important to figure out later.

Rationale and alternatives

In spite of all the ink spilled in this document, the described process is actually very lightweight. It can be summarized as: open a topic in the RFC subforum, argue about it in the comments, and it gets approved or rejected. Most of the RFC Process RFC is just trying to head off forms of dysfunction that have been seen in other places, which includes describing things in excruciating detail, or in some cases even spelling out parts of the process that don't exist.

This process avoids a lot of features that Python PEPs have, such as Editors. Adding a role like that might help reduce the noise that project leadership is exposed to, but that hasn't really been a problem yet, and nobody likes having all their time spent dealing with gatekeeping nonsense.
The other big alternative is to just continue on as-is. Such a choice seems livable, but not really good, for reasons spelled out in Motivation.
This RFC introduces the RFC process first, and postpones creating a core team for later. The alternative would be to try to set up a core team first, and then set up an RFC process. The main reason for going with the RFC-process-first approach is that the RFC process is designed to deal with specific pain points, even under the BDPT model.

Prior art

PEP 8002 does a nice job of examining other project governance systems
The process spelled out in this "RFC process for bors" is particularly modeled after Rust's RFC-0002, with additional references to PEP 0001.
Some changes made here are modeled after stuff discussed in "scaling empathy" and "organizational debt". While this RFC certainly does not attempt to plan a process that will scale like Rust needs (we just aren't that big yet), the process should be adaptable enough, and some tweaks were deliberately made so that Bors can experiment with things that were discussed for use in Rust.
Part of the moderation policy in general is designed to avoid this kind of failure mode.
- Note that Lobsters and Discourse have very different treatments of flagging: in Discourse, if three users with TL1 or better flag a post, the post will be hidden regardless of how many likes it gets. This was an intentional driving force in choosing Discourse instead of, for example, Reddit; I are intentionally taking an "if there's reasonable doubt on whether it's okay, hide it until a moderator comes around" approach.
- The bors approach also specifically avoids trying to take a "self-governing" moderation approach in favor of just biting the bullet and using a hierarchy.
- And, of course, because topics are not sorted by likes, it seeks to encourage discussion rather than groupthink.
Basically, bors is at the first stage.

Unresolved questions

Is this too much?
An RFC only gets a number when it's approved, right?
Should I bother archiving the RFCs in GitHub pages or Amazon S3 or something? It seems like a good idea to have as many backups as possible for these things, but I'm not sure if it's truly necessary.

Future possibilities

The Core Team and The Moderation Team

This is the biggie; instead of just having notriddle check everything, there should probably be a small group who decides whether to approve an RFC or not.

Once the core team exists, it'll probably also be worthwhile to have an automated "proposal for FCP" system like Rust's rfcbot.

RFC automation

In the current implementation, a lot of stuff is done manually upon approving an RFC:

Giving RFCs numbers
Archiving them on GitHub Pages
Creating tracking issues
Un-wiki-fying the initial post

All of these steps can be automated, but it should probably all be done after some experience is had with the process; we don't want to set stuff in stone too early.

RFC tracking

Similarly, it would be really cool to have an automatically-built Kanban board. Perhaps just a GitHub Project generated using the API.

Discussion Final Comment Period Approved Implemented Rejected/Withdrawn Postponed

Discussion	Final Comment Period	Approved	Implemented	Rejected/Withdrawn	Postponed
Write Status to a GitHub Check Minimally-Viable GitLab Support New-Style Emoji Non-Batching Mode	Batch-Size Limit	Localization Support for the Web Site GitLab Worker Protocol Basic REST API	Try Cancellation Command (`bors try-`)	Vape spam Turn bors into a SPA Full-blown issue tracking

Write Status to a GitHub Check

Minimally-Viable GitLab Support

New-Style Emoji

Non-Batching Mode

Batch-Size Limit

Localization Support for the Web Site

GitLab Worker Protocol

Basic REST API

Try Cancellation Command (bors try-)

Vape spam

Turn bors into a SPA

Full-blown issue tracking

Summary: Bors-NG promises to support the current version of Elixir, plus the previous stable release, and the previous two Erlang versions.

Motivation

While bors has been deliberately conservative with non-Mix dependencies, it's never had a written plan for them.

The purpose of supporting older versions of OTP/Elixir is to allow upgrading to be decoupled. They shouldn't have to upgrade everything at the same time. If their OS package manager hasn't approved the new version of OTP, but they're pulling bors from git, they can still stay up-to-date as long as their PM gets around to pushing the update before the next one comes out.

Guide-level explanation

Bors will always be tested against the most recent, and second most recent, feature release of Elixir (feature releases are defined by the second number: 1.8.1 and 1.8.2 are the same feature version). The list of released versions of Elixir can be found at https://github.com/elixir-lang/elixir/releases. Bors will always test with the latest bugfix releases. For example, if 1.7.1, 1.7.2, 1.7.3, 1.7.4, 1.8.1, and 1.8.2 all exist, then bors will test against 1.7.4 and 1.8.2.

Bors will also be tested against the corresponding two most recent feature releases of OTP that are supported by the Elixir versions. For example, when this RFC was written, Bors will support OTP 22.0.1 and 21.3.8.2.

Reference-level explanation

The set of Elixir versions we test against can be found, and updated, in https://github.com/bors-ng/bors-ng/blob/ef4587809ee21600dbbcc6df127c9f6210aa2a1f/.travis.yml and https://github.com/notriddle/docker-phoenix-elixir-test. The latter should probably be moved...

Elixir versions are currently not automatically bumped. Check https://github.com/elixir-lang/elixir/releases and https://github.com/elixir-lang/elixir/releases and http://erlang.org/download/otp_versions_tree.html for the versions to use.

Drawbacks

It slows down development.

Rationale and alternatives

Our main underlying dependency, the Phoenix framework, seems to follow a similar policy. See https://github.com/phoenixframework/phoenix/blob/fb3c92309c02cca1e4b3d857f8f19f27987078d0/.travis.yml for their (somewhat wider) set of supported OTP and Elixir versions.
The exact decision is based on a desire to avoid nonsense where you can't update bors because Chocolatey or Debian haven't gotten around to reviewing a new version of Elixir yet. These package managers tend to either never update, like Debian Stable, or never go more than one version behind upstream, like Debian Unstable or Chocolatey. I don't want to deal with the former, but I don't want people messed up by the latter, hence this choice.
Alternatively, we could actually just pin ourselves to Phoenix ("we support every Elixir version that the corresponding version of Phoenix supports"). I'd like to hear a use case for it, though.

Prior art

These sorts of version pinning is so common, Travis CI has it built-in.
An earlier version of this policy called the previous-stable version "oldstable", after Debian "oldstable", since Debian supports the immediately previous stable version for the same reason (the user needs a chance to upgrade).

Unresolved questions

What happens of some dependency can't handle it? Do we have to fork libraries? Or make an exception? (I vote for temporary forking, but I'm willing to be persuaded otherwise)

Future possibilities

Better guarantees and contracts between the deployment env and bors-ng.

Summary: Clean up GitHub Issue labels.

Motivation

There are a couple of big problems with them.

They're not consistently set. Only people with access to the repository can add them (which is mostly just me, plus a few other regulars). The amount of work required to maintain them, however, should be minimized if possible.
They're not the same as what other stuff uses. GitHub treats the good first issue label a special, but knows nothing about E-easy, which predates it. Dependabot uses plain old elixir and javascript labels, which are redundant with L-elixir and L-javascript. Doing a bunch of bespoke configuration is annoying, and it would be better to integrate with others' defaults.
They're too complicated. The only labels that anyone really cares about are something like "issue accepted", the language labels, E-easy, and maybe a high priority ones. The other schemata is not necessary.

Guide-level explanation

We should change this to just be a redirect:

https://bors.tech/starters/ ➡ https://github.com/bors-ng/bors-ng/contribute

Also:

Reference-level explanation

high priority — indicates an issue that impacts a lot of existing users, has security implications, or otherwise needs done NOW
blocked — indicates a pull request that should not be merged, or an issue that cannot be solved yet
javascript — indicates an issue or pull request that primarily relies on JavaScript code (sometimes added by dependabot to pull requests)
elixir — indicates an issue or pull request that primarily relies on Elixir code (sometimes added by dependabot to pull requests)
css — indicates an issue or pull request that primarily relies on CSS code
html — indicates an issue or pull request that primarily relies on HTML and IEX code
good first issue — indicates that the fix should be easy, self-contained, and that someone is ready to guide you through fixing it
mentored — indicates that someone is ready to guide you through fixing it
help wanted — indicates that the issue is definitely a bug or accepted feature
dependencies — indicates that a pull request updates dependencies (added by dependabot to pull requests)
duplicate — indicates that an issue or pull request was redundant

Drawbacks

This does remove a few labels, particularly the ones distinguishing between feature requests and bugs. This is mostly to avoid pointless categorization between the two, but it does remove some help with prioritizing.

Rationale and alternatives

This design is mostly modeled after what repositories other than Rust do. In particular, several of the labels are based on the ones that Dependabot and GitHub treat specially, so that we get better integration with them.
- https://help.github.com/en/github/building-a-strong-community/encouraging-helpful-contributions-to-your-project-with-labels
- https://help.github.com/en/github/managing-your-work-on-github/about-labels
We could, alternatively, keep the bug and feature labels, if anybody actually uses them.
Dependabot only seems capable of replacing the dependencies label, not of actually changing the language labels, so making it use the L- labels seems impossible.

Prior art

Bazillions of them. Here's some I knew about and had in mind.

https://github.com/phoenixframework/phoenix/labels
https://github.com/rust-lang/rust/labels
https://github.com/elixir-lang/elixir/labels
https://github.com/SergioBenitez/Rocket/labels
https://github.com/libressl-portable/portable/labels
https://github.com/lobsters/lobsters/labels
https://github.com/github/hub/labels

Also, I expect a decent number of users to learn through GitHub's documentation, so let's try to follow their recommendations wherever possible:

https://opensource.guide/

Unresolved questions

We should probably look at the other repos in more detail. I would really like some numbers:

How often are particular labels sorted on?
How often are particular labels applied?
How are labels used? For dev priorities? For newbie discovery?

Future possibilities

I can't think of any.

Implementation status

Yet to implement

Summary: Easily understand where in the implementation process an accepted RFC is.

Motivation

Once a RFC has been accepted, it is generally very common to show a status in some form whether it has been implemented or not. While the archive shows all the accepted RFCs, it does not show what implementation status each RFC is in.

Guide-level explanation

Every RFC in the archive will have a section at the end titled Implementation Status whose text will be one of the following:

Yet to be Implemented - with a link to a github issue calling for it's implementation.
In Review - with a link to github pull request which implements it.
Implemented - with a link to the merged github pull request which implemented it and the date.

In addition, we will have a top-level page on the archive called Implementation Status Summary on which will have a summary of all the implementation statuses of the RFCs.

Drawbacks

The maintainer might feel a bit tedious in managing the statuses of the RFCs. But the status change will happen only once or twice per RFC.

Rationale and alternatives

Instead of doing both a Summary and an Implementation Status on each RFC, we can choose to do only one of them.

Instead of Implementation Status, we can just create an issue calling for the RFC implementation and link it to the RFC leaving the reader to following it properly using Github.

Prior art

IETF RFC 7492

Future possibilities

Automate the RFC process. Once an RFC is accepted, an issue is created automatically and once that is fixed, RFC's implementation Status is updated.

Breaking backwards compatibility, or adding something new that will need to be kept working.

Summary: Commit to maintaining an extension that adds a bors r+ button to GitHub, move it to the bors-ng GitHub org, and publish it to as many extension stores as possible.

Motivation

One of the common complaints about bors-ng is that the command is a pain to type and annoying to remember. There aren't very many good ways to make it easier that don't have significant drawbacks, but what we could do is add a button to the user interface that approves something in bors instead of requiring the user to type it out.

Guide-level explanation

To make using bors with GitHub Reviews easier, you can install the bors button extension. This adds a button to the GitHub Reviews interface to approve-and-merge.

Starting out, this extension defaults to assuming bors-ng is used in its default configuration, but also has a hardcoded list of projects that use instances of homu or bors-ng with customized command names instead. Improving how it figures that out in the future would make a great pull request, but for now, if you're using a setup with bors-ng or homu and the bors r+ button doesn't work for you, you can also open a pull request to add your project to the hardcoded list.

If you have permission to do so, the bors r+ button makes an Approving GitHub Review with bors r+ in it, so not only is bors triggered to run, it also gives that green checkmark to your contributor, which is comforting when they're not familiar with your workflow.

Reference-level explanation

The WebExtension is maintained by a small group of people called the "webextension keyholders." They share access to accounts in every applicable browser extension store (like the Microsoft Edge one, the Google Chrome one, the Apple Safari one, and the Mozilla Firefox one). Adding the bors extension to another store requires there to be at least one keyholder who primarily uses that browser, to make sure it's getting tested in it.

The keyholders team will decide where those keys should be stored. Probably, 1password, keybase, or something.

Drawbacks

This is kind of tangential to the bors-ng project itself. Especially since we're committing to supporting homu users with this extension.

Rationale and alternatives

On the other hand, since we're supporting bors-ng users who reassign their bot name, we already have to have the whitelist, since there's no way to detect what the trigger word is for any particular repository anyway. We could add an API, but that's far more work, and this way seems fine as long as there aren't that many bors-ng instances that change the word.
The impact of doing nothing is that we can't offer an option to people who hate the command interface. They have been, and continue to, ask for ways around it.

Prior art

This is not the only extension for GitHub. There's plenty of them, like ZenHub and GitHub Extended. I'm not aware of any CI apps that do this, though.

Since this is a trivial convenience improvement for the existing app, it should easily fit in with the existing bors-ng workflow. From the point of view of someone not doing the reviewing, nothing should change at all.
As the keyholders group, the concept of using a single user account with a shared password in a password manager is copied directly from what Mozilla does.

Unresolved questions

I'd like to gather together everyone who will be in the extension keyholders group. There are a lot of accounts associated with bors, and it would be nice if we could trial the keyholders' process structure with something that isn't critical infrastructure.
Which browsers will be initially supported depends on who I can convince to help test it.

Future possibilities

Designing an API that we can use instead of the hardcoded list should be done as its own RFC.

Implementaton status

Implemented:

Source code: https://github.com/bors-ng/bors-extension
Firefox add-on: https://addons.mozilla.org/en-US/firefox/addon/bors-for-github-com/

Summary: Provide a way for projects to run arbitrary code before testing, and before merging.

Motivation

Consider an auto-formatter, like clang-format. When should you run it?

You could assume everyone contributing to your project will run it before committing. But this is error-prone and requires changes to many individual users' computers, which isn't practical, especially for open-source projects.
You could use your CI to enforce that the linter has been run: run it on a temporary checkout, see if it makes any changes, and if it does then fail the test and force someone to fix it by hand. But creates extra work for contributors.
You could make a bot that automatically formats PRs, and pushes the changes back into the PR branch. But inserting commits into a PR branch like this causes problems for users when they want to update their PR, because now the PR branch on github doesn't match their PR branch locally. This is annoying, and contributors with less experience with git may not be able to recover at all.

The ideal workflow, therefore, is: approve PR -> merge from master -> auto-format -> test -> merge to master. The "pre-test" hook in this RFC allows bors-ng to handle everything after the approval. And you probably also want to run this during "try" commands too, so we provide a "pre-try" hook as well.

Consider a continuous deployment workflow, where everytime you merge into master, you also want to upload the latest version somewhere. Since bors-ng takes care of merging into master, it seems like a natural place to handle this uploading step. This RFC defines a "pre-merge" hook to handle this.

There's some more discussion in this thread: https://forum.bors.tech/t/running-custom-code-before-and-after-the-merge-for-continuous-deployment-or-other-uses/315

Guide-level explanation

In bors.toml, you can set pre-test, pre-try, and pre-merge hooks to be a list of URLs:

pre-test-hooks = ["https://example.com/pre-test-example"]
pre-try-hooks = ["https://example.com/pre-try-example"]
pre-merge-hooks = [
    "https://example.com/pre-merge-example1",
    "https://example.com/pre-merge-example2",
]

The pre-test and pre-try hooks are invoked after bors-ng has prepared a commit for testing, but before running tests. They may be used to alter the code being tested – for example, by running an auto-formatter. The pre-merge hook is invoked after an approved commit has passed testing, just before bors-ng merges it into the target branch. It cannot alter the code, but it can do other things, like trigger a deployment process or create a tag.

A hook is "invoked" by POSTing a JSON document to the given URL – something like:

{
    "phase": "pre-test",
    "repository": "https://github.com/bors-ng/bors-ng",
    "work-branch": "staging.tmp",
    "target-branch": "master",
    "commit-id": "0b7d6e1ccf9d5fdbce73b690da8a4f76fffc38eb",
    "timeout": 60,
    "callback": "https://bors.tech/webhook/callback/5a4857627"
}

More keys may be added in the future. Unrecognized keys should be ignored.

When implementing a hook, the first thing you should do is validate the payload. Normally this includes the following steps:

Confirm that the request actually came from the bors-ng service, by checking the X-Bors-NG-Signature header. This will contain a comma-separated list of signatures, like:
```
X-Bors-NG-Signature: sha256-hmac=5257a869e7ecebeda32affa62cdca3fa51cad7e77a0e56ff536d0ce8e108d8bd, some-future-algorithm=6ffbb59b2300aae63f272406069a9788598b792a944a07aba816edb039989a39
```
Right now the only supported form is sha256-hmac, which will contain hex-encoded SHA256-HMAC of the request body. The key is a unique per-repo secret, which can be obtained from your bors-ng dashboard. In the future, more algorithms may be added, and you should be prepared to detect and ignore unrecognized signatures.

Remember to use a constant-time equality test whenever checking HMAC signatures. For example, in Python use hmac.compare_digest.
Before manipulating the work-branch, confirm that its HEAD matches the given commit-id. This helps avoid race conditions in case the bors-ng service and your hook get confused and out-of-sync.

Depending on what your hook does, it may also make sense to revalidate this commit id later. For example, if you're going to mutate the branch, don't use git push --force; always use git push or git push --force-with-lease.

The hook responds by POSTing JSON documents to the given callback URL. These documents look like:

{
    "status": "success",
    "comment": "Hook complete - for details see [here](https://...)"
}

The "status" field must be one of "success", "failure", or "pending". The "comment" field contains arbitrary Github-flavored Markdown.

The "timeout" field in the request body indicates how many seconds bors-ng is willing to wait for the callback to be invoked. If bors-ng receives a "pending" response, then it resets its timeout. We recommend that if a hook needs more than this amount of time to run, then it should send a "pending" callback every (timeout/2) seconds to extend the timeout.

Reference-level explanation

Hook lifecycle

Hooks are always invoked in sequence with each other and with other operations on the branch, never in parallel. For example, if there are two pre-test hooks, then bors-ng performs the following steps in order:

Set up the staging.tmp branch
POST to the first hook
Wait for the first hook's callback to be invoked.
POST to the second hook
Wait for the second hook's callback to be invoked
Rename staging.tmp → staging to start the test run.

Rationale: hooks are allowed to mutate things and may fail. Running them sequentially makes it much easier to reason about these cases.

If a hook returns a non-200 response, the timeout expires, the callback is invoked with any status besides "success" or "pending", or the callback payload is invalid, then the run is immediately aborted, and an error is reported to the user, similar to a test failure. If the run is a rollup, then the entire rollup is failed; no attempt is made to bisect the failing commit. (Rationale: hooks are not tests; they're intended to always succeed. So if a hook fails, there's no point in trying to figure out which commit to blame; the hook is to blame, and the hook is what needs to be fixed.)

If a pre-merge hook alters the working branch (i.e., after running it, the head of the test branch has changed), then bors-ng should detect that and report an error rather than merge an untested commit.

Request payload

The request payload is a JSON object with the following keys:

phase: one of "pre-test", "pre-try", or "pre-merge"
repository: the canonical URL for the repository
work-branch: the branch in that repository that bors-ng is working on. Generally "staging.tmp" for pre-test hooks, "trying.tmp" for pre-try hooks, and "staging" for pre-merge hooks.
target-branch: For pre-test and pre-merge hooks, the name of the target branch that this change will eventually be merged into. For pre-try hooks, this is always null.
commit-id: Bors-ng believes that this commit is the HEAD of the work-branch.
timeout: The number of seconds bors-ng will wait before assuming the hook has crashed.
callback: The URL the hook should POST updates to.

Security

We need some way for hooks to tell that they're being invoked by bors-ng, and for bors-ng to tell that the callbacks are coming from the hooks.

The latter is fairly easy: bors-ng should create a new, high-entropy callback URL for each hook invocation. That way it knows that anyone who invokes the callback is in fact authorized to do so.

For invoking hooks, life is more difficult, because we expect hook URLs to be checked into public repositories. And if hooks don't validate that it's really bors-ng invoking them, then anyone who knows the hook URL could send it a POST that pretends to be from bors-ng. If timed correctly, this might cause a hook to try to auto-format a testing.tmp branch that bors-ng is in the middle of assembling, or trigger the deployment of a branch whose tests are still running. That would be bad.

Therefore, each time bors-ng adds a new repo, it generates a 256-bit secret. This secret is shown on the repo's dashboard page, and can be manually regenerated by the user (e.g. by pressing a button on the dashboard).

When bors-ng invokes a hook, it includes a X-Bors-NG-Signature header, containing a SHA256 HMAC of the request body, keyed by this secret: sha256-hmac=<256 bits of hex>. (Prior art: Github, Stripe.)

The header should also contain some fake signatures with randomly generated names, like sdof123-jow=<more gibberish here>. This forces hook implementations to be prepared to handle unrecognized signature algorithms, so that if we later need to transition to a new signature algorithm we can do that without breaking everyone (see also).

Drawbacks

It's kind of complicated? But auto-formatting and continuous deployment are pretty awesome things.

Rationale and alternatives

We could provide a way for bors-ng to run arbitrary code directly. That might be acceptable for locally-hosted versions, or with complex sandboxing. But we would like to support this feature in the publically hosted version, and without complex sandboxing. This is the simplest approach I can think of that doesn't require trusting the hook code.

In practice, it seems likely that the pre-test and pre-try hooks will often be the same. I kept them separate because I'm not sure whether this is always true or not.

The pre-merge hook has a weaker rationale than the others, because there are other ways to invoke code after your tests pass, or after code is merged into master (e.g. the deployment support in most popular CI systems). However, it still seems to be worth including, because:

In the continuous deployment use case, you probably want pre-test and pre-merge hooks to collaborate (e.g., setting the version number in pre-test, and then creating the corresponding tag in pre-merge). Splitting this between two different services seems like a pain.
If you're using multiple status checks and want to gate deployment on all of them together, then that requires some separate service. bors-ng is already set up to do this.
Most hosted CI systems expose deployment secrets to all users with write access to the repository. In bors-ng deployments, its common that there are users who have write permission (e.g. to create branches or modify issues), but who don't have permission to push directly to the master branch. Storing the deployment secrets in some other system, whose access is gated behind bors-ng, is a way to enforce the NRSROSE principle for deployments as well as merges.
We expect that the cost of implementing pre-merge hooks should be pretty low, since they should be able to share most of their code with pre-test/pre-try hooks.

Prior art

I'm not aware of any prior art for these kinds of hooks in other NRSROSE systems.

Unresolved questions

None.

Future possibilities

I briefly considered adding a mechanism to pass information between the pre-test and pre-merge hooks. (For example: the pre-test hook could return some JSON data, which would then be passed through to the pre-merge hook.) I couldn't think of any use cases, so I left it out, but it could be added later if any use cases are discovered.

If you have a lot of repositories managed by bors-ng, then it might be tiresome to have to manually configure secrets for each one. Maybe we'll want to add a way to let multiple repositories use the same secret? (E.g., a per-github-org secret instead of a per-repo secret.)

Implementation status

Yet to be implemented

Summary: Add support for creating merge commits locally in self-hosted instances of Bors. This feature will never be enabled on the free public instance of Bors.

Motivation

Currently, Bors uses GitHub's Git Database API to create merge commits. This approach results in relatively inexpensive disk storage requirements and is sufficient for most purposes.

However, there are several important features that are not possible when creating merge commits with the Git Database API. For example:

Merge commits created using the Git Database API are not automatically GPG-signed (issue #647).
Merges performed using the Git Database API do not respect the merge strategies specified in any .gitattributes files (issue #512).

Guide-level explanation

An instance of Bors can be configured either to perform all merges using the Git Database API or to perform all merges locally using local git. This is a server-wide setting; the same setting will apply to all repositories on the Bors server. The default setting is to perform all merges using the Git Database API.

If you manage an instance of Bors, you can enable local git merges by setting the perform_git_merges_locally configuration setting to true in the config.exs file. The default value of perform_git_merges_locally is false.

# General application configuration
config :bors, BorsNG,
  command_trigger: {:system, :string, "COMMAND_TRIGGER", "bors"},
  home_url: "https://bors.tech/",
  allow_private_repos: {:system, :boolean, "ALLOW_PRIVATE_REPOS", false},
  perform_git_merges_locally: {:system, :boolean, "PERFORM_GIT_MERGES_LOCALLY", true},
  dashboard_header_html: {:system, :string, "DASHBOARD_HEADER_HTML", """
          <a class=header-link href="https://bors.tech">Home</a>
          <a class=header-link href="https://forum.bors.tech">Forum</a>
          <a class=header-link href="https://bors.tech/documentation/getting-started/">Docs</a>
          <b class=header-link>Dashboard</b>
    """},
  dashboard_footer_html: {:system, :string, "DASHBOARD_FOOTER_HTML", """
        This service is provided for free on a best-effort basis.
    """}

By default, Bors will look for an executable named git in the PATH. To customize the location of the git executable, set the value of the git_command configuration setting. The default value of git_command is git.

# General application configuration
config :bors, BorsNG,
  command_trigger: {:system, :string, "COMMAND_TRIGGER", "bors"},
  home_url: "https://bors.tech/",
  allow_private_repos: {:system, :boolean, "ALLOW_PRIVATE_REPOS", false},
  perform_git_merges_locally: {:system, :boolean, "PERFORM_GIT_MERGES_LOCALLY", true},
  git_command: {:system, :string, "GIT_COMMAND", "/bin/git"},
  dashboard_header_html: {:system, :string, "DASHBOARD_HEADER_HTML", """
          <a class=header-link href="https://bors.tech">Home</a>
          <a class=header-link href="https://forum.bors.tech">Forum</a>
          <a class=header-link href="https://bors.tech/documentation/getting-started/">Docs</a>
          <b class=header-link>Dashboard</b>
    """},
  dashboard_footer_html: {:system, :string, "DASHBOARD_FOOTER_HTML", """
        This service is provided for free on a best-effort basis.
    """}

This RFC does not deprecate or break any existing features. Maintainers of Bors servers will always have the option to perform all merges using the Git Database API.

Reference-level explanation

If you were to perform the merge yourself, here are the steps you would follow. Suppose that we have a repository located at https://github.com/someusername/somerepository.git, and we want to merge pull requests #10, #15, and #20 into the target branch sometargetbranch.

git clone https://x-access-token:<token>@github.com/someusername/somerepository.git
cd somerepository
git checkout sometargetbranch
git fetch origin pull/10/head:randomlygeneratedstring-10
git checkout randomlygeneratedstring-10
git fetch origin pull/10/head:randomlygeneratedstring-15
git checkout randomlygeneratedstring-15
git fetch origin pull/10/head:randomlygeneratedstring-20
git checkout randomlygeneratedstring-20
git checkout sometargetbranch
git branch --force staging
git checkout staging
git merge randomlygeneratedstring-10 randomlygeneratedstring-15 randomlygeneratedstring-20
git push --force origin staging
cd ..
rm -rf somerepository

In order to implement this in Bors, we'll likely need to write additional versions of the start_attempt method in attemptor.ex and the start_waiting_batch and start_waiting_merged_batch methods in batcher.ex. Then, we'll add logic that calls the appropriate methods based on the value of the perform_git_merges_locally configuration option.

Drawbacks

In order for Bors to perform a merge locally, the server on which Bors is running will need to have sufficient disk space for git to be able to clone repositories locally. Thus, any Bors server maintainer that is considering setting perform_git_merges_locally to true should make sure that they will have enough disk space.

Let N denote the number of repositories on which Bors is installed, and let M denote the size on disk of the largest repository. Since each repository could have both bors r+ and bors try commands running simultaneously, a conservative estimate for the disk space requirement for cloning repositories is 2MN.

Rationale and alternatives

The alternative will be to continue using the Git Database API for creating merge commits. This is a great option for most users, and it will continue to be the only option available on the free public instance of Bors. However, as described above, there are certain features that will never be available through the Git Database API, and for those, a Bors administrator will need to use local merges.

Prior art

The following apps implement the NRSROSE and have the option to create the merge commit locally:

Unresolved questions

How do we implement tests for this feature?

Future possibilities

Next steps for maintainers of Bors servers after this feature is implemented

GPG-signing all merge commits

If you run a Bors server and you want to enable GPG-signing of all merge commits, you first need to set perform_git_merges_locally to true. Then, you should log in to the server as the same user that Bors runs as and run the following commands:

git config --global user.signingkey your_gpg_key_id
git config --global commit.gpgsign true

Using specific merge strategies for certain files

If you use a Bors server with perform_git_merges_locally set to true, and you want to use a specific merge strategy for certain files in one of your repositories, you only need to create an appropriate .gitattributes file and commit it in your repository. The command-line git will use the .gitattributes file automatically.

Future work in Bors

Once the ability to create merge commits locally has been implemented, future work might include the implementation of a git_squash = true option in bors.toml that would pass the --squash flag to the git merge command. This could potentially help us close issue #138 and/or issue #194.

But this would be an advanced feature, and we should not implement it as part of this first RFC. The best plan is to first implement the ability simply to create merge commits locally. Once we have implemented that, we can consider opening a second RFC for discussion specifically of a git_squash = true option for passing the --squash flag to the git merge command, or something similar.

Implementation status

Yet to be implemented

Summary: Implement a configuration option for bors to produce "squash merge" commits. When this option is enabled, instead of attaching all commits under the approved pull requests to a batch merge commit, bors will generate one, single, commit for each approved pull request.

Motivation

Currently, Bors merges in the entire commit history of a PR. This ends up polluting the git history with huge numbers of small commits that don't matter. Github solves this by offering a "Squash and merge" functionality. This will compress the entire commit history into a single commit and provide a good commit title with a link back to the PR.

Squash and merge functionality is widely used in the Github community and is a blocker for rolling out Bors to more locations.

There are several open bugs/feature requests that want the functionality of a squash merge in Bors. Eg: https://forum.bors.tech/t/is-anyone-working-on-squash-merge-for-bors/340 and https://github.com/bors-ng/bors-ng/issues/194

Guide-level explanation

To have Bors use the Github "squash-and-merge" functionality.

Edit your bors.toml file and add use_squash_merge = true. (The default value of use_squash_merge is false.)

When you run bors r+, bors will operate as normal, except when your batch passes:

All changes in each PR will be squashed together, so there is one commit per PR.
The commit message will be set to use the PR title with a link to the PR, and the the commit details will contain the commit messages that have been squashed together.

Reference-level explanation

The basic functional loop of this feature is:

Create the staging branch
If the use_squash_merge flag is set to true for the report then
For each Patch

Get the Git tree of the final commit in each patch
Create a commit on the staging branch with the commit tree (this is a squash commit)
Generate a commit title from the PR title
Generate a commit text body from the PR description
Create a merge commit (if needed) of all squashed commits

Run the regular Bors build checks
When a batch passes all required checks - For each Patch in a Batch

Change the title of the patch from "$title" to "[Merged By Bors] - $title"

Drawbacks

This is an opt-in feature. I think most people would get a better experience if it was just enabled by default.

Rationale and alternatives

Why is this design the best in the space of possible designs?

This change leaves all other aspects of the code-base alone. The functionality is exclusively opt-in, and does not require any additional services, process, or resources to be installed in the Bors container. The code changes are non-obtrusive

What other designs have been considered and what is the rationale for not choosing them?

One alternative is using the green button merge functionality. This works but could allow some code to be merged without being fully validated against the tests.

What is the impact of not doing this?

This is a highly demanded feature and not implementing it will hamper adoption of Bors in the wider world.

Prior art

See https://forum.bors.tech/t/is-anyone-working-on-squash-merge-for-bors/340 and https://github.com/bors-ng/bors-ng/issues/194 for prior discussions of this.

Future possibilities

The alternative option could become easy to support if Github adds a more robust Git manipulation API.

Implementation status

Implemented:

Pull request: https://github.com/bors-ng/bors-ng/pull/718

Summary: Add an option to have bors parse and enforce the GitHub-designed CODEOWNERS file.

Motivation

Currently, bors only supports requiring numbers of contributors. This works for many use cases but several users need the power of GitHub CODEOWNERS. CODEOWNERS let users set a 'required' code reviewer for specific code. Eg, you could require review by security team if code in the security repository is altered.

Guide-level explanation

This feature is intended to exactly mimic the functionality GitHub provides with their CODEOWNERS implementation. As such, the exact guide level docs are owner by GitHub(here)

For bors, CODEOWNERS file support will be enabled by setting enable_codeowners = true in the bors.toml file.

Reference-level explanation

Please see Github official docs

Drawbacks

This will somewhat complicate the system by requiring us to follow the CODEOWNERS file format instead of just counting required reviewers.

Rationale and alternatives

This is an important tool to support the Bors Squash merge feature. Many teams cannot switch to squash-merge if they would lose the CODEOWNERS functionality.

An alternative, this could be implemented by an external github tagger bot. The bot could label PRs that still need review and could block on the attached label.

Prior art

GitHub - This feature is widely used in larger organizations.

Unresolved questions

Should the feature simply be automatically enabled when a CODEOWNERS file is present?

Future possibilities

We could extend the CODEOWNERS functionality to support features not officially supported in GitHub proper.

Implementation status

Implemented:

Pull request: https://github.com/bors-ng/bors-ng/pull/725

Summary: Add a new configuration option status_wait_success. Bors will wait until all of the GitHub Statuses listed in status_wait_success succeed (i.e. "turn green"), or until Bors times out.

Motivation

There are some GitHub Statuses that will initially report a failing (i.e. "red X") status, but at a later point in time will change their status to passing (i.e. "green checkmark"). An example would be the codecov/project status from CodeCov. If you have multiple CI jobs that each report partial coverage, then initially the codecov/project status will be failing. However, after all of the CI jobs have completed and submitted their coverage reports, CodeCov will merge all of the coverage reports and will change the codecov/project status to passing.

If you list such a status in the status section of your bors.toml file, then Bors will fail the build as soon as it sees a failing status. But this is not what we want - we want Bors to ignore those failing statuses and instead wait until the status passes (or until Bors times out, whichever comes first).

A more detailed description of the CodeCov use case is available in this forum thread.

Guide-level explanation

If you want Bors to require that a certain GitHub Status pass, and if you want Bors to immediately fail the build if that GitHub Status ever reports a failing (i.e. "red X") status, put that status in the status list.

If you want Bors to require that a certain GitHub Status pass, and you want Bors to wait until that GitHub status passes (i.e. "green checkmark"), and you do not want Bors to fail the build if that GitHub Status ever reports a failing status, put that status in the status_wait_success.

If you specify one or more statuses in the status_wait_success list, the Bors will wait until all of those statuses succeed. If not all of the statuses have succeeded by the time that the Bors timeout period has elapsed, then Bors will fail with a timeout error.

Example

For example, suppose that you want Bors to require that the following four statuses pass in order for Bors to merge:

continuous-integration/travis-ci/push
Taskcluster (push)
codecov/project
codecov/patch

If the continuous-integration/travis-ci/push and/or Taskcluster (push) statuses fails, you want Bors to immediately fail the build. However, if the codecov/project and/or codecov/patch statuses fails, you want Bors to ignore the failure, and instead wait until both the codecov/project and codecov/patch statuses succeed. In this example, your bors.toml file would look like this:

status = [ "continuous-integration/travis-ci/push", "Taskcluster (push)" ]

status_wait_success = ["codecov/project", "codecov/patch"]

Customizing the timeout

The default Bors timeout is one hour (i.e. 3600 seconds). You can specifying a longer or shorter timeout by setting the timeout_sec option in bors.toml. For example, to set the Bors timeout to twelve hours (i.e. 43200 seconds), you would set timeout_sec = 43200. So your bors.toml file would look like this:

status = [ "continuous-integration/travis-ci/push", "Taskcluster (push)" ]

status_wait_success = ["codecov/project", "codecov/patch"]

timeout_sec = 43200

Reference-level explanation

Here is a rough overview of the changes that we need to make.

First, we will modify the code that lives in lib/github/github.ex, starting at line 224. Currently, the code looks like this:

@spec map_state_to_status(binary) :: tstatus
def map_state_to_status(state) do
  case state do
    "pending" -> :running
    "success" -> :ok
    "failure" -> :error
    "error" -> :error
  end
end

@spec map_check_to_status(binary) :: tstatus
def map_check_to_status(conclusion) do
  case conclusion do
    nil -> :running
    "success" -> :ok
    _ -> :error
  end
end

@spec map_status_to_state(tstatus) :: binary
def map_status_to_state(state) do
  case state do
    :running -> "pending"
    :ok -> "success"
    :error -> "failure"
  end
end

We will modify it to look like this:

@spec map_state_to_status(binary) :: tstatus
def map_state_to_status(state) do
  case state do
    "pending" -> :running
    "success" -> :ok
    "failure" -> :error
    "error" -> :error
  end
end

@spec map_check_to_status(binary) :: tstatus
def map_check_to_status(conclusion) do
  case conclusion do
    nil -> :running
    "success" -> :ok
    _ -> :error
  end
end

@spec map_status_to_state(tstatus) :: binary
def map_status_to_state(state) do
  case state do
    :running -> "pending"
    :ok -> "success"
    :error -> "failure"
  end
end

@spec map_state_to_status_wait_success(binary) :: tstatus
def map_state_to_status_wait_success(state) do
  case state do
    "pending" -> :running
    "success" -> :ok
    "failure" -> :running
    "error" -> :running
  end
end

@spec map_check_to_status_wait_success(binary) :: tstatus
def map_check_to_status_wait_success(conclusion) do
  case conclusion do
    nil -> :running
    "success" -> :ok
    _ -> :running
  end
end

@spec map_status_wait_success_to_state(tstatus) :: binary
def map_status_wait_success_to_state(state) do
  case state do
    :running -> "pending"
    :ok -> "success"
    :error -> "pending"
  end
end

Basically, we want Bors to think of any status other than success as pending.

Then we will add logic that checks the bors.toml file and calls the correct functions (i.e. map_state_to_status versus map_state_to_status_wait_success) depending on whether the relevant GitHub Status was listed in status or status_wait_success.

We should also make it a bors.toml parsing error to specify the same status in both status and status_wait_success.

Drawbacks, rationale and alternatives

The drawbacks, rationale and alternatives have been explained in detail in this forum thread.

The feature described here is the only approach that avoids a race condition.

Prior art

I am not aware of any prior art.

Unresolved questions

How will we implement tests for this new feature?

Future possibilities

I can't think of any future possibilities at this time.

Implementation status

Yet to implement

Summary: Add bors d+ as an alias to bors delegate+ and bors d=someone as an alias to bors delegate=someone.

Motivation

Based on @tommilligan's pull request and @matklad's issue, a lot of people are annoyed by how long the delegate command's name is.

Guide-level explanation

In addition to adding reviewers who can approve any PR in the repo, you can “delegate” permission to approve a single PR to anyone else. It works like this:

    @some-user: bors r+
    @bors[bot]: Permission denied
    @some-reviewer: bors d=some-user
    @bors[bot]: some-user now has permission to review this pull request.
    @some-user: bors r+
    @bors[bot]: Added to queue

If some-user happens to be the pull request author, you can also use the shorthand d+ command.

Reference-level explanation

Syntax	Description
`bors delegate+`	Allow the pull request author to r+ their changes.
`bors delegate=[list]`	Allow the listed users to r+ this pull request’s changes.
`bors d+`	Allow the pull request author to r+ their changes (same as `delegate+`).
`bors d=[list]`	Allow the listed users to r+ this pull request’s changes (same as `delegate=[list]`).

Drawbacks

It adds another command that we need to maintain.

It's also less readable, though, since bors responds with a message with what you're doing, that should be fine. Also, we already allow the super-short and kind of opaque r+, so it doesn't make much sense to be picky about self-documenting commands.

Rationale and alternatives

Why is this design the best in the space of possible designs?

It fits with the way r+ and r= work.
What other designs have been considered and what is the rationale for not choosing them?

About the only one that's really been considered is leaving it with delegate.
What is the impact of not doing this?

It's kind of annoying for people who regularly use delegation.

Prior art

No known prior art for this specifically. As an interesting note, the reason why the command keyword is bors instead of bors-ng, or the older name of this codebase, aelita2, is for this same reason, so there's lots of precedent for using short command names in bors.

Unresolved questions

The "design" is pretty much already nailed down.

Future possibilities

ping is extremely rare, and pretty short anyway, so there's no real point in abbreviating it. We also couldn't use p anyway, since that's already taken for priority adjustment.

The only other multi-letter commands are try and retry. Try is pretty short, at three letters, and retry ought to be pretty rare, even if not as rare as ping.

Implementation status

Implemented

Pull request: https://github.com/bors-ng/bors-ng/pull/727

Summary: Add bors merge as an alias to bors r+, bors merge=FOO as an alias to bors r=FOO, bors merge p=123 as an alias to bors r+ p=123, bors merge=FOO p=123 as an alias to bors r=FOO p=123, and bors merge- as an alias to bors r-.

Motivation

Based on the discussion in a previous forum thread (https://forum.bors.tech/t/user-defined-aliases-for-bors-commands/371), multiple users are confused by the bors r+ command.

Guide-level explanation

Once you've set it up, instead of clicking the green "Merge Button", leave a comment like this on the pull request:

Equivalently, you can comment the following

The pull request, as well as any other pull requests that are reviewed around the same time, will be merged into a branch called "staging". Your CI tool will test it in there, and report the result back where Bors-NG can see it. If that result is "OK", master gets fast-forwarded to reach it.

The status can be seen in the Dashboard page, which also makes a good one-stop-shop to see pull requests that are waiting for review.

Reference-level explanation

Syntax	Description
bors r+	Run the test suite and push to master if it passes. Short for "reviewed: looks good."
bors merge	Equivalent to `bors r+`
bors r=[list]	Same as r+, but the "reviewer" in the commit log will be recorded as the user(s) given as the argument.
bors merge=[list]	Equivalent to `bors r=[list]`
bors r-	Cancel an r+ or r=.
bors merge-	Equivalent to `bors r-`
bors p=[priority]	Set the priority of the current pull request. Pull requests with different priority are never batched together. The pull request with the bigger priority number goes first.
bors r+ p=[priority]	Set the priority, run the test suite, and push to master (shorthand for doing p= and r+ one after the other).
bors merge p=[priority]	Equivalent to `bors r+ p=[priority]`

Drawbacks

It adds another command that we need to maintain.

It creates multiple ways of doing the same thing, which might lead to some confusion.

Rationale and alternatives

Why is this design the best in the space of possible designs?

It allows for consistency across different projects that use Bors-NG.

What other designs have been considered and what is the rationale for not choosing them?

We could allow unlimited customization of aliases in bors.toml. But then this will lead to inconsistency across different projects that use Bors-NG and will become a real headache for people that are reviewers on multiple different projects.

What is the impact of not doing this?

If we do nothing, then people will continue to be confused by the opaque r+ command.

Prior art

The prior art for adding aliases to Bors-NG is the following RFC: https://forum.bors.tech/t/allow-delegate-to-be-abbreviated-as-d/362

Unresolved questions

None.

Future possibilities

Hopefully we won't need to add any more aliases after this one.

Implementation status

Implemented:

Pull request: https://github.com/bors-ng/bors-ng/pull/746

Summary: We distinguish between Github statuses that are errors and waiting differently (instead of treating them all as a failed passed_status . In the case where there are no errors and some waiting statuses, notify with a response and wait polling with an exponential backoff until we're in some other state than "no errors and some waiting statuses" (i.e., patch_preflight returns something else).

Motivation

When using the pr_status feature to filter allowed pull requests, it really makes more sense to actually wait for it to finish, rather than immediately erroring. This process can also time out, so it is also necessary to expose a configuration option for changing that timeout.

Guide-level explanation

There are two ways to gate on a GitHub Status CI report. The regular status option is used to watch for CI runs on the staging branch. The other, pr_status, is used to watch for integrations that only run on pull requests. Bors will wait for the PR status to complete before it merges into the staging branch.

Reference-level explanation

Change the documentation for pr_status. Also add documentation for this configuration option.

Name	Description
`pr_status`	List of commit statuses that must pass on the PR commit ~~when it is r+-ed~~ before it will go into staging. If it is still running, bors will wait. If it fails, bors will fail immediately.
`prerun_timeout_sec`	Timeout while waiting for `pr_status` checks to finish.

Drawbacks

Other than code complexity, there are really no user-facing reasons to avoid doing this. It's obviously better in the cases where a PR status is still running. There are no known cases where users would prefer the old behavior, really.

Rationale and alternatives

Not doing this is a pretty terrible option that got a lot of complaints before someone eventually got around to implementing it.
As for doing this with a different setup, the obvious alternative is to add a separate config option for "insta-fail statuses" and "waited statuses". If anybody wants that, let me know!

Prior art

Homu doesn't seem to have anything even remotely comparable. The pr_context config option that they have never triggers waits. It triggers bypass behavior.
Zuul seems to work the same way this RFC and its corresponding PR behave, though I can't tell for sure from their docs. They have a bunch of different kinds of triggers in place, so it might really be project-specific. Their dependency infrastructure definitely seems sophisticated enough to implement this RFC, though.

Unresolved questions

The question that came to my mind, that I'd like to know about, is the one described in the "alternatives" section: does anybody have any status changes that they don't want bors to ever wait on? That is, does anyone actually prefer the old behavior?

Future possibilities

There are a bunch of pre-checks. Right now, we only poll statuses, but maybe we'd want to poll some of the others? Even if it's only with super-short timeouts to try to work around potential race conditions.

Implementation status

Implemented:

Pull request: https://github.com/bors-ng/bors-ng/pull/785

Summary: Allow patches to be batched by themselves only.

Motivation

This RFC is to allow users to configure batch size limit by providing an option to allow patches to be batched by themselves only.

Guide-level explanation

Appending "single on"or "single=on" to the bors review command will force a patch to be batched by itself only and will never be put it a batch with other patches nor will other patches join its batch, like so:

Once a patch is reviewed with "single on" appended, all future reviews on the patch will remember the option and the patch will always be batched alone.

If this behavior is no longer desired, it can be disabled by appending "single off" or "single=off" to the review command and can be re-enable later by appending "single on" to a review command.

Additionally, the single command may be used without a review command like to turn the behavior on or off without having to review.

Reference-level explanation

The following options to the existing "bors r+" command, along with its alias, will be added:

These commands will enable and disable the functionality to force a patch to always be batched by itself. Because the option is appended to the review command, once the option is handled, the review command will also be handled.

To support the option, the "patches" table will require a column to store the flag which will be checked before batching the patch.

Checking whether a patch should be batched alone will be performed during the handling of the review command and during the bisection of a batch. Batches that are currently running or waiting will be left alone.

Drawbacks

There shouldn't be any drawbacks since the option only affects patches which have received the specified command and can be disabled if desired.

Rationale and alternatives

This design is a simple solution to providing the users a method to limit batch size.
An alternative solution would be allow the user to specify the max number of patches a batch can hold which would affect all patches in a project.

Prior art

The original bors system used implemented this solution. Unsure if other CI/CD systems implemented this system.

Unresolved questions

Is it necessary for the bisecting logic to honor the "single on" option? If the patch is already in a batch, it would either be merged in without any issues or fail sooner or later. Additionally, since the option is appended to the review command, the patch shouldn't ever be seen by the bisecting logic unless the patched r+'ed again while it's already in a batch.
Should the option also handle patches that are already in a running/waiting batch? Should it pull out the patch and place it in a separate batch by itself?

Future possibilities

None foreseen.

Implementation status

Implemented:

Pull request: https://github.com/bors-ng/bors-ng/pull/839

The required_approvals option should enforce that every piece of code that makes it into the master branch, has been reviewed by the requisite number of people. However, the current implementation counts approving reviews even if new code has been added to a PR since the review. This unfortunate situation requires users to manually reason about which code has been adequately reviewed. Bors should recognize approvals only if they apply to all of the changes which are going to be merged. For backwards compatibility and flexibility, this should be controlled by a new configuration option.

Motivation

Suppose you have a project which specifies required_approvals = 3:

Person A creates a PR.
Reviewers B, C, and D approve it.
Person A pushes an "extra" commit to the PR.
Reviewer B runs bors r+.

In this situation, bors will happily merge the PR, even though the extra commit wasn't actually approved by anyone except for possibly reviewer B. This makes the required_approvals configuration option much less useful, since it fails to ensure that all changes have been approved by 3 people.

The invariant that required_approvals = n means every piece of code that makes it into the master branch has been reviewed by n people is a natural extension of the NRSROSE, and bors should enforce it too.

Guide-level explanation

The configuration option up_to_date_approvals = true causes GitHub Reviews to count towards the number of required_approvals only when they are up to date. Pushing new changes to the PR branch requires prior approving reviews to be resubmitted in order to count.

Enabling up_to_date_approvals makes certain that any code merged to master has been adequately reviewed.

Reference-level explanation

up_to_date_approvals is a boolean configuration option in bors.toml defaulting to false.

We say review R is up to date for commit hash X if R's commit_id key as seen through the GitHub Review API is exactly X. Otherwise it is stale. If up_to_date_approvals is true, then whenever the number of approving reviews is counted for the purpose of comparing against required_approvals, only those reviews which are up to date for the hash of the commit to be merged are counted. If up_to_date_approvals is false, then all reviews from the PR are counted (maintaining current behavior). The up_to_date_approvals option has no effect if required_approvals is not set. The up_to_date_approvals option also does not affect the determination of whether or not there are any rejecting reviews.

If a bors command is rejected due to not having enough approving reviews, then bors will include the number of reviews counted and the number of reviews required in its comment. If up_to_date_approvals is true, it will also include the number of approving reviews that were not counted because they were not up to date.

Let n be the value of required_approvals, m be the number of total approvals present, and l be the number of up to date approvals.

If up_to_date_approvals is false, or if l == m:

If up_to_date_approvals is true and l != m:

Drawbacks

Adds a new configuration option, increasing complexity.
Might require more API calls to determine if reviews are up to date.
UX may be confusing since GitHub will still display the stale reviews, but bors won't count them.

Alternatives

Make up_to_date_approvals = true the default behavior or the only behavior.
- Not chosen because it would be a breaking change for current workflows
- Not all users may want this stricter behavior, since it requires more effort from reviewers and change authors
Use a CI check to enforce the number of up-to-date reviews
- It's more impactful to make this change in bors instead of requiring every project that wants this behavior to implement it themselves.
- Requires duplicating most of the existing functionality of required_approvals just to change one small aspect of it.
Do nothing
- For some projects it's not acceptable for code changes to be merged without a certain minimum number of reviews.

Prior art

The GitHub option, "Dismiss stale pull request approvals when new commits are pushed.", essentially implements the same behavior as this.
- Note that projects using bors can't take advantage of this, because GitHub doesn't allow enabling this option without also enabling the "Require [nonzero number] of approving reviews" protection rule on the target branch. This latter rule is itself incompatible with bors, since the merge commit created by bors counts as "unapproved".
The Gerrit approval system automatically dismisses reviews when new code is pushed.
graydon/bors' implementation of code review uses approval comments which are either attached to the commits themselves, or with the commit hash included in the comment. Either way, approvals only count for a single commit. This is the implementation used by the Rust language project.

Unresolved questions

What parts of the design do you expect to resolve through the RFC process before this gets accepted?
- Final choice of configuration option name and exact wording of error messages
What parts of the design do you expect to resolve through the implementation of this feature before deployment?
- Details of interaction with the GitHub API to determine which reviews are stale.
What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC?
- Dismissal of stale rejecting reviews

Future possibilities

Didn't think of anything.

Implementation status

Implemented:

Pull request: https://github.com/bors-ng/bors-ng/pull/986

When a PR depends upon another one that's been merged, update the unmerged PR base branch to point to the merged PRs base branch.

Motivation

The current behaviour causes dependant PRs to be closed then the merged branch is deleted. Github doesn't allow editing the PR or re opening it: a new PR needs to be created, losing any discussion that was already in the original.

Guide-level explanation

Supponse we have 2 PRs A (base branch default) and B (base branch a) and A is merged by bors. If bors.toml contains update_base_for_deletes = true, then bors will update B to have base default before deleting branch a.

Reference-level explanation

An optional setting update_base_for_deletes (default false) will be added to bors.toml to enable/disable the feature.

When the branch deleter determines it should delete the branch for a merged/closed PR closed_pr, it will first query the Github PR API for any pull request that have base set to closed_pr.head_ref. It will then update the base to point to closed_pr.base using the Github API again.

If use_squash_merge is disabled, then the updated PR will show a message with the base branch change, but nothing else will change since github ignores commits that already exist on default. Otherwise, if squash merges are enabled, the closed PR commits will be listed but the changes ignored in the UI and by git for the next squash merge.

Drawbacks

Adds a new configuration option, increasing complexity.
Github UI will be confusing if use_squash_merge is enabled

Rationale and alternatives

Why is this design the best in the space of possible designs? This is not a one-size-fits-all solution, since workflows can exist that would be made harder by the proposed behaviour. Hiding the feature behind a flag allows users to opt in when they fell they'd reap the benefits.

What other designs have been considered and what is the rationale for not choosing them? Stop bors from deleting branches if PRs exist with it as base. While this reduces the manual work required by developers by avoiding the need to create a new PR, it also introduces a potential foot gun. Those PRs can inadvertently be merged onto a branch that is already considered merged, causing devs to think their changes have been integrated on the default branch when they weren't. While you could argue that updating the base branch is also a potential foot gun since you can merge onto the default branch without realizing, I can't think of a workflow where I'd want to merge changes onto an already integrated branch.

What is the impact of not doing this? Manual work is required whenever a PR with others depending on it is required, and review context is lost each time that ocurrs. In our case, this caused team members to stop using bors altogether due to the friction to their workflows.

Prior art

If you use Github to merge PRs, it does this automatically by default.

Unresolved questions

What parts of the design do you expect to resolve through the RFC process before this gets accepted?
Final choice of configuration option name since the current one doesn't carry the full meaning
Whether it would be desired to implement the behaviour where bors doesn't delete a branch if PRs point to it can be implemented regardless of this feature.

Future possibilities

Github is previewing an API to rebase a PR that could be used after updating the base of a PR, potentially cleaning up the commit history of the PR.

Implementation status

Implemented:

Pull request: https://github.com/bors-ng/bors-ng/pull/1023

Summary: Add a config option to log all outgoing HTTP requests.

Motivation

This is a debugging aid, which is why it'll be off by default.

Guide-level explanation

If you set the BORS_LOG_OUTGOING environment variable, it will write every HTTP request to the log, making it easier to debug the GitHub API usage. For example

2020-08-03T07:00:15.168988+00:00 app[web.1]: 07:00:15.168 pid=<0.344.0> [info] GET https://api.github.com/repositories/284424834/contents/bors.toml -> 200 (194.823 ms)
2020-08-03T07:00:15.420605+00:00 app[web.1]: 07:00:15.420 pid=<0.344.0> [info] GET https://api.github.com/repositories/284424834/issues/9/labels -> 200 (226.039 ms)
2020-08-03T07:00:15.621675+00:00 app[web.1]: 07:00:15.621 pid=<0.344.0> [info] GET https://api.github.com/repositories/284424834/commits/5a99b398b8c1d16f1ba5120b6670fdb31903c38f/status -> 200 (200.326 ms)
2020-08-03T07:00:16.079808+00:00 app[web.1]: 07:00:16.079 pid=<0.344.0> [info] GET https://api.github.com/repositories/284424834/commits/5a99b398b8c1d16f1ba5120b6670fdb31903c38f/check-runs -> 200 (457.674 ms)
2020-08-03T07:00:16.286852+00:00 app[web.1]: 07:00:16.286 pid=<0.344.0> [info] GET https://api.github.com/repositories/284424834/commits/5a99b398b8c1d16f1ba5120b6670fdb31903c38f/status -> 200 (206.264 ms)
2020-08-03T07:00:16.468819+00:00 app[web.1]: 07:00:16.467 pid=<0.344.0> [info] GET https://api.github.com/repositories/284424834/commits/5a99b398b8c1d16f1ba5120b6670fdb31903c38f/check-runs -> 200 (180.530 ms)
2020-08-03T07:00:16.468828+00:00 app[web.1]: 07:00:16.467 pid=<0.547.0> [info] Checking code owners
2020-08-03T07:00:16.667539+00:00 app[web.1]: 07:00:16.666 pid=<0.344.0> [info] GET https://api.github.com/repositories/284424834/contents/.github/CODEOWNERS -> 200 (198.168 ms)
2020-08-03T07:00:16.687920+00:00 app[web.1]: 07:00:16.686 pid=<0.547.0> [info] CODEOWNERS file %BorsNG.CodeOwners{patterns: [%BorsNG.FilePattern{approvers: ["@organisationjetteauloin/dummy"], file_pattern: "*"}]}

Reference-level explanation

The BORS_LOG_OUTGOING environment variable will activate the Tesla.Middleware.Logger plug-in.

Drawbacks

It's probably not going to be used that much, though the ones that do use it will love it.

Rationale and alternatives

This could be turned on by default, instead of requiring a config option, but it's not likely to be used much in the production instances, because it's very noisy.
This could be done using a different method than the Elixir logger, but maybe metrics gathering should be a separate matter anyway.

Prior art

This is based on Tesla's existing features. We're just exposing it to the user.

Unresolved questions

Do we want something this ad-hoc? Or would we rather come up with more of an all-encompassing plan?

Future possibilities

Collecting metrics on API calls could allow us to detect things that go wrong, even in production setups where "just log and read everything" isn't much of an option.

Implementation status

in progress

This RFC proposes a new environment variable, PUBLIC_PROTOCOL. If unset, or set to https, the behaviour of bors will remain unchanged, and the bors app will be served at a HTTPS endpoint. If set to http, the bors app will be served at a HTTP endpoint.

Motivation

Allow the bors app to run at a HTTP endpoint if HTTPS is not required or available.

Specifically, I have a use case at work to serve the bors app at a HTTP endpoint, which will be connected to securely by other means (SSH port forwarding).

Currently, the PUBLIC_HOST is assumed to be served over HTTPS (and delivers 301 redirects accordingly). This makes it impossible to expose the app at a HTTP address.

12:46:59.521 pid=<0.2186.0> [info] Plug.SSL is redirecting GET / to https ://172.17.0.1:39103 with status 301

This also makes it hard to test changes locally, without proxying via another service acting as a TLS terminator (such as nginx).

Guide-level explanation

A new environment variable PUBLIC_PROTOCOL will be added, to match the existing variables PUBLIC_PORT and PUBLIC_HOST.

This can be left unset, or set to one of https, http. The behaviour of https and unset are identical, and consistent with current behaviour.

If set to http, the bors app will be served at a HTTP endpoint.

Please see the proposed implementation (link below)

Reference-level explanation

No further detail.

Drawbacks

This gives users the option to expose bors at a HTTP endpoint. This conceivably makes it easier for users to expose such a deployment insecurely. However, this feature is opt it, requiring a user to consciously consider their choice.

Rationale and alternatives

This design is back compatible, and allows more flexibility when choosing to deploy bors.

If bors does not have an option to serve the dashboard over HTTP, this makes deploying bors strictly harder in some scenarios.

Prior art

It is common for developer-oriented dashboards and internal access points to be exposed over HTTP, with HTTPS an external concern.

For instance:

CockroachDB Dashboard: https://www.cockroachlabs.com/docs/v20.1/admin-ui-overview#admin-ui-access
Elasticsearch's Kibana: https://www.elastic.co/guide/en/kibana/current/access.html#access

Unresolved questions

n/a

Future possibilities

n/a

Motivation

My team uses the conventional commits specification (conventionalcommits .org) for our commit messages and we use GitCop (gitcop .com) to police that users adhere to these rules. We also only merge PRs using Bors, but Bors's hardcoded commit titles do not comply to the same rules.

I would like to have a way for us to specify what the commit title should look like when Bors creates a merge commit. This feature has also been mentioned in issue 637 and discussed in https://forum.bors.tech/t/how-to-change-the-commit-message-format/307.

Guide-level explanation

I propose to introduce a new configuration property: commit_title.

The commit_title configuration property can be used to define a custom template for the commit title, that Bors applies to generate the commit message of new merge commits. The commit_title can be configured in your bors.toml configuration file.

For example, let's say you'd like Bors to merge all PRs with the commit title merge pull request. Simply add the following to your bors.toml configuration file.

commit_title = "merge pull request"

It is also possible to refer to the PR's unique identifier using a reserved keyword in the commit_title.

For example, let's say you'd like Bors to merge a PR with id #8 using the commit title merge: #8. Just add the following to your bors.toml configuration file.

commit_title = "merge: ${PR_REFS}"

If you do not specify a commit_title, it defaults to:

commit_title = "Merge ${PR_REFS}"

Reference-level explanation

The commit_title is defined in the following BNF:

<commit_title> ::= <segment> | <commit_title> <segment>
<segment> ::= <prefix> <pr_refs> <suffix>
<prefix> ::= "" | <string>
<pr_refs> ::= "" | "${PR_REFS}"
<suffix> ::= "" | <string>
<string> ::= ... # any non empty string

This grammar means that the commit_title can contain:

any non-empty string
as many ${PR_REFS} references as you'd like, and in any index in the string

All occurrences of the ${PR_REFS} keyword are replaced by the string of pr references. This is generated in the following manner:

For every PR that will be merged with this commit:
   take the identifier (i.e. `xref`)
   prepend the char `#`
   join with space char in between

For the set of PR identifiers [1,2,3,4] this would result in: #1 #2 #3 #4.

Drawbacks

Why should we not do this?

It adds an additional config property and a reserved keyword to maintain.
It might lead to unforeseen bugs.

Rationale and alternatives

Why is this design the best in the space of possible designs? It allows for flexibility and is backwards compatible
What other designs have been considered and what is the rationale for not choosing them? I have not yet considered other designs
What is the impact of not doing this? Some users might feel they can't use a tool like Bors if they can't have this feature, although unlikely.

Prior art

For feature RFC's: What other CI/CD systems implement anything similar to this one? What do they do well? What do they do poorly? If this proposal is similar to what one of them already does, did you change anything, and why or why not? Why would the user pick bors over just using that other thing instead?

Rultor does not support this feature. I don't know any other tools that do the same.

Also for feature RFC's: How does this feature you're proposing complement the software development lifecycle of real teams? Compare what you're implementing to things that they're doing by hand, or using other tools. Please make sure that this feature is likely to be used by more than one project.

Other users have requested this feature.

For process RFC's: What do other major open-source projects do? Focus on ones that make their environment welcome to marginalized groups. If they have a reputation for being mean, we don't want to copy them.

What lessons can we learn from what other communities have done here? Why do they do what they do? Why is it applicable to bors, or why isn't it?

Papers: Are there any published papers or great posts that discuss this? If you have some relevant papers to refer to, this can serve as a more detailed theoretical background.

Unresolved questions

What parts of the design do you expect to resolve through the RFC process before this gets accepted?

Specify and commit to a grammar and naming of the config property

What parts of the design do you expect to resolve through the implementation of this feature before deployment?

What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC?

Commit body customisation Additional reserved keywords for PR summary and description

Future possibilities

Commit body customisation Additional reserved keywords for PR summary and description

Motivation

Depending on the way bors is deployed, the default timeout might be too short.

Guide-level explanation

If your deployment requires it, you can change the database timeout, such as in your docker-compose.yml

DATABASE_TIMEOUT: 60000

Reference-level explanation

The new environment variable:

DATABASE_TIMEOUT may be set higher than the default of 15_000(ms). This may be necessary with repositories with a very large amount of members.

We recommend also trying to make sure you have a high POOL_SIZE if you’re going to do this, so that other queries don’t get blocked, and also using a slow query log to try to find and optimize whichever query or transaction is taking so long.

Drawbacks

It might just be masking a problem, such as a missing index or a transaction that’s blocking on an API call.

Rationale and alternatives

It’s pretty much just exposing Ecto’s timeout setting. I’m not sure how else it would work.

We could also try to avoid hitting the lower timeout by modifying the code, or by dedicating a better database server to it, but that requires lots of work and profiling.

Prior art

Basically everybody exposes this kind of configurable knob. It’s weird that bors-ng didn’t.

Unresolved questions

Beyond bikeshedding the name? Nothing really.

Future possibilities

We expose API and database timeouts. How about incoming HTTP timeouts?

Motivation

Right now, bors checks whether a pull request is "not mergeable" according to GitHub's API, before it tries to merge it. This is good, but it's kinda late at that point. The pull request webhook actually reports whether it's mergeable in advance, so if bors used that, it would be able to reject it earlier and avoid wasting time, and it could show that information on the pull request dashboard.

Guide-level explanation

This change is almost completely transparent to the user, and would basically only change the behavior in very weird corner cases, like if they queued a pull request that wouldn't merge cleanly into the main branch behind another one that actually fixed the merge conflict.

Reference-level explanation

Bors will check whether your pull request is mergeable both before enqueueing it, and after dequeueing it.

Bors will also refuse to merge draft pull requests.

Drawbacks

It adds two new columns to the database, and one new column to the dashboard.

Rationale and alternatives

By adding it to the database, bors can also show it in the dashboard, which makes it a bit more useful (whether a pull request is mergeable or not seems important).
The current one uses slightly less code, but it's probably not what we want.

Prior art

It's the design that homu uses.

Unresolved questions

I'm not really aware of any.

Future possibilities

Right now, we don't pluck unmergeable pull requests out of batches, but probably could change to do that. I don't think it would actually make much difference, performance-wise, but it would make the dashboard a bit more accurate.

Motivation

It's just readability of the chat history.

Guide-level explanation

Use bors cancel to cancel a PR from the queue.

Reference-level explanation

command	description
bors cancel	Equivalent to bors r-

Drawbacks

We already have one synonym. There's not much reason not to have two.

Rationale and alternatives

Custom commands? No way. It's hard enough making sure everyone understands what's going on.

Prior art

bors merge-

Unresolved questions

What's the cut-off? We don't want to enable every possible one. cancel seems alright, because other parts of the app call it "canceling," but if someone proposes "stop," we might want to say no.

Future possibilities

Since we already refer to it as "cancel"ing in other places, this makes sense, but beyond that, there hopefully won't be any more of these.

Motivation

Support load balancers, proxies, and other services that implement some form of health checks without requiring workarounds like treating redirects as healthy or bypassing the health checks altogether. Some systems don't allow the above workarounds, and cannot be used. This health check endpoint is not meant to display any details about the status of Bors features or the database, simply provide a way for services to verify the underlying Elixir server is ready to handle requests.

Guide-level explanation

The /health endpoint can be used to verify the Elixir server is running and ready to accept requests.

Reference-level explanation

Added endpoint(s):

/health - will always return an HTTP 200 OK response code. This endpoint does not imply that all features are functioning as expected, instead it simply states the Elixir server powering Bors is running and ready to handle requests.

Drawbacks

It could be misleading to have a health check endpoint that returns HTTP 200s when things internally might be broken. It's also an additional endpoint, albeit a simple one, that will require maintenance and consideration moving forward.

Rationale and alternatives

Not really sure of any alternatives here besides bypassing health checks altogether, or updating specific load balancers/proxies to support treating redirect behaviors as healthy but that would be significantly more difficult and in some cases just outright impossible for hosted service like GCP or AWS.

Prior art

It seems this essentially does what we need, however it's behind a redirect and is specific to webhooks. https://github.com/bors-ng/bors-ng/blob/08a938d08bdad80b84199d99511f62479be6d012/lib/web/controllers/webhook_controller.ex#L90-L92

Unresolved questions

I've assumed the endpoint would live at /health, but am open to considering different routes.

Future possibilities

This health check endpoint could theoretically be expanded in the future to support reporting data for Bors internals, such as queue size.

Motivation

As part of a company initiative to improve our security and manage better our costs we have moved our bors-ng deployment from Heroku into one of our private Kubernetes clusters. As part of that effort we have experimented and generated some Terraform code and Helm charts to achieve the same goal which is getting bors-ng running on a Kubernetes cluster. Since all this effort is not bounded in any way to the company we have decided to donate these Terraform snippets and Helm chart to bors-ng since we are benefitting from the application.

This is specially useful for operator (like myself)

Guide-level explanation

The code to add is not related to the application bors-ng on per se but the way it's run on the container orchestrator Kubernetes. A PR (link at the bottom) has already been created. The code included on the PR contains:

Helm chart v2 (Helm 3 compatible only)
Terraform snippet to deploy on Kubernetes using kubernetes provider
Terraform snippet to deploy on Kubernetes using helm provider and the chart included here

Reference-level explanation

As explained before this RFC does not affects the bors-ng code instead adds a couple of ways to easily deploy bors-ng on Kubernetes using some of the most popular tooling to manage infrastructure and Kubernetes

Drawbacks

More code to maintain

¯\_(ツ)_/¯

Rationale and alternatives

Why is this design the best in the space of possible designs?
What other designs have been considered and what is the rationale for not choosing them?
What is the impact of not doing this?

Prior art

Heroku one click deployment
docker cli command to run bors on a container

Unresolved questions

Publishing the chart on a Helm repository. An option has been suggested on the PR mentioned below using Helm chart releaser and Github Pages making the hosting free and automated

Future possibilities

Adding bors-ng Helm repository to CNCF Artifacthub (searchable index for many Helm chart repositories)
Helm chart compatible with Openshift
Terraform modules addded to the hashicorp registry
This could help to improve bors-ng adoption

The Bors RFC Archive