Skip to content

Conversation

@friederbluemle
Copy link
Contributor

@friederbluemle friederbluemle commented Sep 20, 2021

A couple of commits here:

  1. Update and simplify CI script (e.g. replace deprecated bash script with codecov-action)
  2. Run yarn test --coverage to generate and publish coverage data

Also another important improvement: Prevent running yarn dv-scripts ci on forks. Obviously, the step will fail in forked repos, as the secrets are not available in that context and of course you wouldn't want to publish from forks anyway.

Example of successful build/coverage report:

https://github.com/friederbluemle/react-native-fast-image/runs/3645227154
https://codecov.io/gh/friederbluemle/react-native-fast-image/tree/a0808cb138ed9e71fd40f1e21dee6dbec52042dd

@@ -0,0 +1,18 @@
name: ci
on: [push]
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
on: [push]
on: [push, pull_request]

It would be good to see that PRs are passing, I think this would make it work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh.. actually now I remember why I didn't add it. PR builds also run in the context of the source repo, but don't contain the secrets (e.g. NPM_TOKEN), for obvious security reasons. Also we shouldn't release with every new PR build (yarn dv-scripts ci).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I addressed this by changing the conditional to:

if: github.event.repository.fork == false && github.event_name != 'pull_request' 
Copy link
Owner

Choose a reason for hiding this comment

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

I think I have the rethink how dv-scripts works a bit to simplify this 😆 . Good for now though!

@DylanVann
Copy link
Owner

Thanks, this looks great!

@codecov
Copy link

codecov bot commented Sep 21, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@4aea52f). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@ ## main #826 +/- ## ======================================= Coverage ? 96.29% ======================================= Files ? 1 Lines ? 27 Branches ? 2 ======================================= Hits ? 26 Misses ? 1 Partials ? 0 

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4aea52f...af985d9. Read the comment docs.

@DylanVann DylanVann merged commit d96b851 into DylanVann:main Sep 23, 2021
@friederbluemle friederbluemle deleted the update-ci branch September 23, 2021 13:58
@github-actions
Copy link

🎉 This PR is included in version 8.5.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

anaisbetts added a commit to chatterbugapp/react-native-fast-image that referenced this pull request Sep 28, 2021
* upstream/main: release(version): Release 8.5.11 [skip ci] fix: null exception in FastImageViewManager.java (DylanVann#423) release(version): Release 8.5.10 [skip ci] chore: convert examples to hooks (DylanVann#830) release(version): Release 8.5.9 [skip ci] fix: FastImage extends ViewProps (DylanVann#829) ci: update CI and (re)enable coverage (DylanVann#826) [skip ci] release(version): Release 8.5.8 [skip ci] docs: fix gifs release(version): Release 8.5.7 [skip ci] chore: upgrade example to react-native@0.65.1 release(version): Release 8.5.6 [skip ci] fix: make corresponding flow file for .cjs file
tungxuan1656 pushed a commit to tungxuan1656/react-native-fast-image that referenced this pull request Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 participants