ci: add github actions #372

bpas247 · 2019-08-25T05:15:35Z

Relevant to 4324 in the React-Bootstrap repository.

Adds github actions, with a workflow for running tests.

NOTE We would need to need to setup a github secret for this to work properly: CODECOV_TOKEN.

Relevant to 4324 in the React-Bootstrap repository. Adds github actions, with a workflow for running tests and a workflow for release. For the release workflow, there is an integration with semantic-release. We would need to need to setup two github secrets for semantic to work properly: `NPM_TOKEN` and `GH_TOKEN`.

since GitHub Actions is being integrated, it replaces the need for Travis CI.

also removes an unneeded dependency.

Was failing due to how windows' line endings work, which made the formatter angry.

also removes an uneeded dependency, as its use case is being handled by the github action.

mxschmitt · 2019-08-26T06:22:00Z

I would not use the codecov Action, because we then fetch the dependencies multiple times and rum the tests multiple times. The codecov npm package works great :)

bpas247 · 2019-08-26T06:35:53Z

I would not use the codecov Action, because we then fetch the dependencies multiple times and rum the tests multiple times. The codecov npm package works great :)

Sure, but if the concern is that it will upload the test coverage multiple times, then it could just be solved by having it run as a separate job (which is how I organized it here, as it re-runs the tests and uploads the coverage results after the main test job completes with a success).

taion · 2019-08-26T14:54:05Z

But Codecov works just fine when you upload the coverage multiple times. It merges the coverage reports into a single combined report.

This is actually useful when different test runs exercise different code branches, e.g. if you have an "if Firefox" vs "if Chrome" branch in the code.

bpas247 · 2019-08-26T15:09:44Z

But Codecov works just fine when you upload the coverage multiple times. It merges the coverage reports into a single combined report.

Oh that's cool, so it's probably not worth running it as a separate job then. We should just bake it into the main testing suite job then.

CodeCov merges all uploaded reports into a single report, so it's not an issue to upload the report multiple times. We still need to define the GitHub secret for CodeCov in order for this to work.

mxschmitt · 2019-08-26T15:30:37Z

I would still not use the codecov action. We only need to execute npx codecov and it will be uploaded. No need for spawning another Docker container (https://github.com/codecov/codecov-action/blob/master/Dockerfile). Both are doing actually the same except, that in the Docker container the bash script of codecov will be ran, which detects the language etc. In the end the codecov Action has more pitfalls.

bpas247 · 2019-08-26T15:39:22Z

I would still not use the codecov action. We only need to execute npx codecov and it will be uploaded. No need for spawning another Docker container (https://github.com/codecov/codecov-action/blob/master/Dockerfile). Both are doing actually the same except, that in the Docker container the bash script of codecov will be ran, which detects the language etc. In the end the codecov Action has more pitfalls.

So I'm gonna have to disagree with you, because if you look at the CI build logs for the react-bootstrap PR then you'd see that even though the code coverage is failed to be reported, the CI build still passes. Using the GitHub action in this PR actually catches the CI build failing when it's not able to upload the report.

I thought about it, and it doesn't really make sense for this PR to try and accomplish two different things. The semantic-release integration should be in an another PR, so removing it for now.

mxschmitt · 2019-08-27T08:20:52Z

I would still not use the codecov action. We only need to execute npx codecov and it will be uploaded. No need for spawning another Docker container (https://github.com/codecov/codecov-action/blob/master/Dockerfile). Both are doing actually the same except, that in the Docker container the bash script of codecov will be ran, which detects the language etc. In the end the codecov Action has more pitfalls.

So I'm gonna have to disagree with you, because if you look at the CI build logs for the react-bootstrap PR then you'd see that even though the code coverage is failed to be reported, the CI build still passes. Using the GitHub action in this PR actually catches the CI build failing when it's not able to upload the report.

True, I've submitted a fix for it here: codecov/codecov-node#144

package.json

jquense · 2019-08-27T16:41:29Z

I'm not sure semantic-release is the best idea. I've used it a few times on other projects and it's ok, but tends to be kinda of annoying to deal with, especially around prereleases and manual releases.

How is authentication working here? whose npm token is going to be used? It also precludes us from requiring 2FA for publishing which is maybe a good idea

bpas247 · 2019-08-27T20:56:20Z

I'm not sure semantic-release is the best idea. I've used it a few times on other projects and it's ok, but tends to be kinda of annoying to deal with, especially around prereleases and manual releases.

Agreed, which is why it was removed as of 45fcbee.

How is authentication working here? whose npm token is going to be used? It also precludes us from requiring 2FA for publishing which is maybe a good idea

We don't need that npm token anymore :)

We aren't integrating semantic-release anymore, so these config options need to be reverted.

jquense · 2019-08-27T20:58:52Z

oops sorry missed it was gone. thanks github :P

bpas247 · 2019-08-27T21:03:13Z

README.md

+[codecov-badge]: https://codecov.io/gh/react-bootstrap/react-overlays/branch/master/graph/badge.svg
+[test-badge]: https://github.com/react-bootstrap/react-overlays/workflows/test/badge.svg


I'm pretty sure this badge link doesn't actually work. @mxschmitt do you mind suggesting what the correct link should be?

I'm not reverting the changes made to the lint script, since that was for the GitHub actions testing ci builds.

The CI was unfortunately skipping builds that weren't linux containers with jobs that had a CodeCov step (which is very strange behavior). Running it as a separate job should fix this issue.

Having to install google chrome externally for macOS build was causing a checksum mismatch error, so removing it now until we can figure out what is causing it.

Since it can only be ran on linux containers (sigh), we need this safeguard in place.

bpas247 added 6 commits August 24, 2019 22:09

chore(ci): remove travis config file

4110b5b

since GitHub Actions is being integrated, it replaces the need for Travis CI.

ci(release): modify semantic-release config to also deploy docs

347ccf4

also removes an unneeded dependency.

fix(ci): potential fix for linting issue on windows ci build

a54f2b6

fix(ci): add git attributes that prevents formatter from failing

b343193

Was failing due to how windows' line endings work, which made the formatter angry.

chore(readme): add github actions badges and remove tavis badge

8d94c91

bpas247 marked this pull request as ready for review August 25, 2019 09:10

bpas247 added 2 commits August 25, 2019 02:13

fix(readme): incorrect naming for github actions badge

e4b47b8

ci(actions): use codecov github actions for uploading coverage

e488540

also removes an uneeded dependency, as its use case is being handled by the github action.

bpas247 added the enhancement label Aug 26, 2019

bpas247 requested review from taion and mxschmitt August 26, 2019 02:24

mxschmitt mentioned this pull request Aug 26, 2019

New release #294

Closed

bpas247 added 2 commits August 26, 2019 08:11

fix(ci): merge upload test coverage step into main testing job

0671fa8

CodeCov merges all uploaded reports into a single report, so it's not an issue to upload the report multiple times. We still need to define the GitHub secret for CodeCov in order for this to work.

fix(ci): container action can only be run on linux containers fix

c0be34e

mxschmitt mentioned this pull request Aug 26, 2019

ci: added GitHub Action workflow for CI react-bootstrap/react-bootstrap#4324

Merged

chore(ci): remove semantic-release integration

45fcbee

I thought about it, and it doesn't really make sense for this PR to try and accomplish two different things. The semantic-release integration should be in an another PR, so removing it for now.

bpas247 changed the title ~~ci: add github actions and integrate semantic-release~~ ci: add github actions Aug 27, 2019

jquense reviewed Aug 27, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

chore(release-config): revert changes to original release config

f5361d5

We aren't integrating semantic-release anymore, so these config options need to be reverted.

chore(readme): remove unused github action release badge

8a02345

bpas247 commented Aug 27, 2019

View reviewed changes

bpas247 added 3 commits August 27, 2019 14:09

chore(package): revert changes made from semantic-release config

dd1fd53

I'm not reverting the changes made to the lint script, since that was for the GitHub actions testing ci builds.

fix(ci): run upload test results as a separate job

d601e7e

The CI was unfortunately skipping builds that weren't linux containers with jobs that had a CodeCov step (which is very strange behavior). Running it as a separate job should fix this issue.

fix(ci): remove macOS CI build

b2f7225

Having to install google chrome externally for macOS build was causing a checksum mismatch error, so removing it now until we can figure out what is causing it.

This comment has been minimized.

Sign in to view

taion approved these changes Aug 28, 2019

View reviewed changes

bpas247 added 2 commits August 27, 2019 17:36

fix(ci): add safeguard for running CodeCov on non-linux container

b29b594

Since it can only be ran on linux containers (sigh), we need this safeguard in place.

fix(readme): correct badge link

85ad01e

bpas247 merged commit bd5c6eb into master Aug 28, 2019

bpas247 deleted the ci/add-github-actions branch August 28, 2019 00:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: add github actions #372

ci: add github actions #372

bpas247 commented Aug 25, 2019 •

edited

Loading

mxschmitt commented Aug 26, 2019

bpas247 commented Aug 26, 2019

taion commented Aug 26, 2019

bpas247 commented Aug 26, 2019

mxschmitt commented Aug 26, 2019 •

edited

Loading

bpas247 commented Aug 26, 2019

mxschmitt commented Aug 27, 2019

jquense commented Aug 27, 2019

bpas247 commented Aug 27, 2019

jquense commented Aug 27, 2019

bpas247 Aug 27, 2019

This comment has been minimized.

		[codecov-badge]: https://codecov.io/gh/react-bootstrap/react-overlays/branch/master/graph/badge.svg
		[test-badge]: https://github.com/react-bootstrap/react-overlays/workflows/test/badge.svg

ci: add github actions #372

ci: add github actions #372

Conversation

bpas247 commented Aug 25, 2019 • edited Loading

mxschmitt commented Aug 26, 2019

bpas247 commented Aug 26, 2019

taion commented Aug 26, 2019

bpas247 commented Aug 26, 2019

mxschmitt commented Aug 26, 2019 • edited Loading

bpas247 commented Aug 26, 2019

mxschmitt commented Aug 27, 2019

jquense commented Aug 27, 2019

bpas247 commented Aug 27, 2019

jquense commented Aug 27, 2019

bpas247 Aug 27, 2019

Choose a reason for hiding this comment

This comment has been minimized.

bpas247 commented Aug 25, 2019 •

edited

Loading

mxschmitt commented Aug 26, 2019 •

edited

Loading