Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update CI status badge in readme #5583

Merged
merged 2 commits into from
Nov 8, 2020
Merged

Update CI status badge in readme #5583

merged 2 commits into from
Nov 8, 2020

Conversation

pierlon
Copy link
Contributor

@pierlon pierlon commented Nov 8, 2020

Summary

Replaces Travis CI status badge with the status badge for the "Build, test & measure" GitHub Action workflow.

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@pierlon pierlon added the WS:Core Work stream for Plugin core label Nov 8, 2020
@pierlon pierlon added this to the v2.1 milestone Nov 8, 2020
@pierlon pierlon self-assigned this Nov 8, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2020 •
edited
Loading

Plugin builds for 05d55f2 are ready 🛎️!

pierlon
pierlon commented Nov 8, 2020
View reviewed changes
.editorconfig
@@ -14,5 +14,5 @@ indent_style = tab
indent_style = space
indent_size = 2

[{*.md}]
[*.md]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This rule didn't seem to be matching as PHPStorm kept removing the trailing whitespace. Also, the linter was giving the error:

image

Once the braces were removed PHPStorm stopped removing the trailing whitespace.

readme.md
@@ -12,7 +12,7 @@ The Official AMP Plugin, supported by the AMP team. Formerly Accelerated Mobile
**License:** [GPLv2 or later](http://www.gnu.org/licenses/gpl-2.0.html)
**Requires PHP:** 5.6

[![Build Status](https://travis-ci.org/ampproject/amp-wp.svg?branch=develop)](https://travis-ci.org/ampproject/amp-wp) [![Coverage Status](https://img.shields.io/codecov/c/github/ampproject/amp-wp/develop.svg)](https://codecov.io/gh/ampproject/amp-wp) [![Built with Grunt](https://gruntjs.com/cdn/builtwith.svg)](http://gruntjs.com)
[![Build Status](https://github.com/ampproject/amp-wp/workflows/Build,%20test%20&%20measure/badge.svg)](https://github.com/ampproject/amp-wp/actions?query=branch%3Adevelop+workflow%3A%22Build%2C+test+%26+measure%22) [![Coverage Status](https://img.shields.io/codecov/c/github/ampproject/amp-wp/develop.svg)](https://codecov.io/gh/ampproject/amp-wp) [![Built with Grunt](https://gruntjs.com/cdn/builtwith.svg)](http://gruntjs.com)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment the badge label is "Build, test & measure". Customizing the label is not possible but it has been reported on GitHub's community forums as a requested feature.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, the status is for the default branch (develop).

@westonruter
Copy link
Member

I updated the require status checks to pass before merging to remove the space.

Just E2E is failing, probably for no good reason. I wonder: should we allow E2E to fail (for now) and make it not required since it seems extremely fragile and we plan to improve the infrastructure used to run it?

@pierlon
Copy link
Contributor Author

pierlon commented Nov 8, 2020

should we allow E2E to fail (for now) and make it not required since it seems extremely fragile and we plan to improve the infrastructure used to run it?

We could, but I'd recommend keeping a close eye on it as Gutenberg does ship breaking changes from time to time.

@pierlon
Copy link
Contributor Author

pierlon commented Nov 8, 2020

Marked E2E job as allowed to fail in 05d55f2.

@westonruter
Copy link
Member

We currently have two jobs for PHPUnit with PHP 7.4, one with coverage and one without:

image

Is this not redundant?

@pierlon
Copy link
Contributor Author

pierlon commented Nov 8, 2020

The one with coverage supplies Coveralls with the PHP coverage. Previously with Travis that job only ran on the develop branch.

@pierlon
Copy link
Contributor Author

pierlon commented Nov 8, 2020

I suppose we could get rid of the non-coverage one.

@pierlon
Copy link
Contributor Author

pierlon commented Nov 8, 2020 •
edited
Loading

Ah wait, the non-coverage one also ran the external-http tests, while the one with coverage did not.

@westonruter

This comment has been minimized.

Sign in to view
@westonruter westonruter merged commit 5534545 into develop Nov 8, 2020
@westonruter westonruter deleted the add/gha-ci-badge branch November 8, 2020 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@westonruter westonruter westonruter approved these changes

Assignees
No one assigned
Labels
WS:Core Work stream for Plugin core
Projects
None yet
Milestone
v2.1
Development

Successfully merging this pull request may close these issues.
2 participants
@pierlon @westonruter