Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Coveralls badges #3477

Merged
merged 1 commit into from
Jul 23, 2022
Merged

Coveralls badges #3477

merged 1 commit into from
Jul 23, 2022

Conversation

kubk
Copy link
Collaborator

@kubk kubk commented Jul 23, 2022

The PR brings coveralls.io badges back. I've also removed Typescript/Prettier badges because nowadays these tools are de facto standard. Let's avoid excess information to not overwhelm reader.

Regarding allowing coveralls job to fail on CI, there is no clean solution yet, the issue is still open: actions/runner#2347

For example, one of the workarounds actions/runner#2347 would allow to merge a PR with optional failed job, but GitHub will show the whole PR as failed (with ❌ symbol).

@urugator How often do you encounter coveralls random fails?

Here coveralls job failed because the tests failed too: be29cd0

@changeset-bot
Copy link

changeset-bot bot commented Jul 23, 2022

⚠️ No Changeset found

Latest commit: 0e61055

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR


[![CircleCI](https://circleci.com/gh/mobxjs/mobx-react-lite.svg?style=svg)](https://circleci.com/gh/mobxjs/mobx-react-lite)
[![Coverage Status](https://coveralls.io/repos/github/mobxjs/mobx-react-lite/badge.svg)](https://coveralls.io/github/mobxjs/mobx-react-lite)
[![NPM downloads](https://img.shields.io/npm/dm/mobx-react-lite.svg?style=flat)](https://npmjs.com/package/mobx-react-lite)[![Minzipped size](https://img.shields.io/bundlephobia/minzip/mobx-react-lite.svg)](https://bundlephobia.com/result?p=mobx-react-lite)
[![Discuss on Github](https://img.shields.io/badge/discuss%20on-GitHub-orange)](https://github.com/mobxjs/mobx/discussions)
Copy link
Collaborator Author

@kubk kubk Jul 23, 2022

Choose a reason for hiding this comment

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

https://github.com/mobxjs/mobx/tree/main/packages/mobx-react-lite#readme

Badges are too close to each other here (unlike in other READMEs in Mobx repo), I've added a bit of space

@urugator
Copy link
Collaborator

Do we know why it fails? To be clear I dunno if badges are always the cause. It seems quite random, so I thought it's because of race conditions or someting, but if it's just some service being unresponsive, then there is not much we can do I guess (besides increasing timeout).

@kubk
Copy link
Collaborator Author

kubk commented Jul 23, 2022

@urugator Sorry for confusing you. The badges have nothing to do with CI. They just query some information from an endpoint and display it.

Regarding CI failures, I saw a few times how Coveralls Github action failed due to 504 from Coveralls API. Yes, there isn't much we can do because it seems like GitHub doesn't have a way to make job optional. In GitLab you can allow job to fail and still get the green pipeline: https://docs.gitlab.com/ee/ci/yaml/#allow_failure

Let's keep an eye on CI to understand how often we encounter such a situation. Then if it fails often, we can apply the workaround described in the PRs description. It should allow to merge even when optional job fails, but we'll still see the red X which might be a source of confusion.

@kubk kubk merged commit 05b5315 into main Jul 23, 2022
@kubk kubk deleted the coveralls branch November 16, 2022 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants