Set up Travis to build the peps

[brettcannon]

That way we can make sure there are no errors.

Not sure if there's a way to make warnings into errors, but if there is then we should do that.

[brettcannon]

We can also require the build pass before accepting a PR, although @dstufft suggests we don't require the PR to be up-to-date (which makes sense since most PRs will be contained to a single PEP).

[dstufft]

Yea, two important things about requiring a build to pass before accepting a PR:

• Requiring passing status checks means that all changes must go via PR, there's no more pushing directly to master.
• If you require the PR to be up-to-date, it means every time a PR is merged, all other PRs need to be rebased or have master merged into them, which is a massive drag, so not requiring them to be up to date makes it easier to land changes with only a small window for breakage.

[dstufft]

Oh, and specific to the first one, you can allow administrators to merge a PR even if things aren't passing (they get a "Are you sure?" prompt) as an escape hatch for the build being broken and needing multiple PRs to fix it.

[holdenweb]

As if :-)

Sent from my iPhone

On 15 Jun 2016, at 23:56, Donald Stufft [email protected] wrote:

Oh, and specific to the first one, you can allow administrators to merge a PR even if things aren't passing (they get a "Are you sure?" prompt) as an escape hatch for the build being broken and needing multiple PRs to fix it.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[birkenfeld]

#17 is a first step, but needs to be further refined. For example, it should be ensured that markup warnings in RST PEPs should make the build fail, which I don't think the current Makefile allows.

[gvanrossum]

Can somebody do something simple here? The discussion seems to have petered out with no clear conclusion. I'd say the better is the enemy of the good. I just want to see whether a PEP builds without warnings before merging it, since doing this manually is a huge pain.

[dstufft]

@gvanrossum We're currently using Travis to run make on every PR and settings a failing status if that returns a non zero exit code. I think that means that warnings will get a passing check but failure to builds will not. You can see the output of running make by clicking on the details of the travis build (for example with #39 if you clicked on "Show all checks" then "details" next to Travis, you'd get https://travis-ci.org/python/peps/builds/140609559 and can scan that for warnings).

If we want to make it so that Travis will fail on warnings as well as on errors, then we'll need to expose a way in the Makefile / pep2html.py to ask it to turn warnings into errors.

[dstufft]

#45 will ensure that even warnings fail the build and not just errors.

[gvanrossum]

I see, cool. It should be simple enough to make a change to pep2html.py to
do exit(1) on warnings, right? Or is there no way to get the warning status
from docutils/rest?

I'm not sure what the point is of making the test required. In the other
projects I'm familiar with (asyncio, mypy) we don't do this and I don't
think it has caused any problems.

[dstufft]

Making the tests required achieves a few things:

• It prevents accidental pushes to master since all changes then need to go through the PR process, this is particularly crummy when someone pushes something broken directly to master.
• When you don't require status checks, Github doesn't show them until the status check has recorded some status ("pending", "success", "failed", or "error") but if you require them then it shows pending immediately. This means people don't have to go and look for which status checks passed to ensure that all of them have actually been reported but they can immediately see that there are still more status checks that haven't reported yet.
• It helps ensure, that for whatever reason, master always stays passing.

[fgregg]

What is left on this issue now that #17 and #45 have been merged?

[brettcannon]

Thanks for pointing out the staleness of this issue, @fgregg ! I added a Travis badge to close out this issue.