Readthedocs.org: Add Configuration for PR Builder Through Config File

[saadmk11]

So, travis does this in the config file to allow users to skip the CI on some branches.

• Do we want to offer that level of customization?
• do we have cases where the user only wants to report the status on some branches?
• Disabling the PR builder setting means don't report the status and not build the docs from the PR or just not report the status?

_Originally posted by @stsewd in https://github.com/readthedocs/readthedocs.org/issues/6014#issuecomment-516533506_

Continuing from the initial comment by @stsewd, We want some kind of customization through the config file for External versions. Maybe for enabling or disabling PR Builds for a project or Disabling for a certain PR.

We need to discuss this issue and figure out how we want the users to interact with the PR Builder. My initial opinion is we add PR Builder Enabling and Disabling feature in the config file for a project. and Also we can do something like this for skipping specific PR Building #3457

Need the @readthedocs/core Teams thoughts on this :)

[ericholscher]

I think the config file feels like the right place for this. If we don't have a strong opinion, I think that copying what travis and others do here probably makes the most sense. So allowing it to be turned on in the config, specifying which branches to build against. The main downside is that it requires us to clone the code in order to figure out if we need to trigger a build, which feels a bit complicated.

[stsewd]

So, just putting more context here from the slack discussion.

Currently, we parse the config file after clone (this is so we can support manual import).
Given that, we can't put any setting that modifies if the PR should be build or not.

One option is having a global setting to enable the feature, and to have control over all pull requests, we can use the automation rules (extend them to support external versions and run the rules before building the PRs)

An action for the rules is given a pattern, only build PRs that match that pattern.

Other action can be build PR but not report status (codecov has this feature, we don't wait for other check in the list, but we can review the coverage report anyway).

[stsewd]

Also related to https://github.com/readthedocs/readthedocs.org/issues/2593

[pllim]

Hello. I really like this feature and I have begun to roll it out to my projects. So, thank you very much!

In my workflow, I find something like [skip rtd] in the commit message to prevent it from running is more useful. This way, PR authors themselves can control it from commit to commit, without having to tinker with config files unrelated to the PR. This is akin to [skip ci] on Travis, Appveyor, or CircleCI. Most of the PRs I deal with are against master branch, so I don't have strong feelings about supporting a per-branch setting.

[humitos]

One option is having a global setting to enable the feature, and to have control over all pull requests, we can use the automation rules (extend them to support external versions and run the rules before building the PRs)

I may be lacking some context here, but I think that having everything in the same place is the way to go. That's it "Automated Rules" in my mind.

There you can control what versions (internal or external) to build, activate, make stable, etc. I think it makes the UX clearer in the sense that you control everything from there instead of "some" from there and some others from the config file.

Besides, we do not need to build all the version/branch name parsing/logic again. We already have it for the Automated Rules.

As an extra point, allows us to even do not trigger the build at all and save resources.

Sounds like a win-win (users-us) to me.

[ericholscher]

Yea, I think my main concern is that we're moving configuration out of the YAML file again, and into the database. The whole goal was to try and keep as much logic in the YAML as possible, and now we're drifting back the other direction :/

[agjohnson]

I tend to agree that the best UX is in automation rules, and also want to be selective about what we're putting into the config file. Project level options are a good fit for the config file. This seems like a project level setting because what do we do when the config file in one branch specifies a different list of branches to exclude? Only using the config from the default branch seems like a bit of a hack compared to building out automation rules.

[humitos]

Project level options are a good fit for the config file.

I remember from our old conversations that we said "Version level options are good for Config File and Project level are good candidates for DB".

I understand @ericholscher's point, but I think that it's preferable to have this _global_ settings in the DB to avoid triggering a build and having different settings on different versions that it's suppose to affect the Project globally.