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

Setup basic GitHub Actions CI #4

Merged
merged 2 commits into from
Mar 24, 2021
Merged

Setup basic GitHub Actions CI #4

merged 2 commits into from
Mar 24, 2021

Conversation

rwjblue
Copy link
Collaborator

@rwjblue rwjblue commented Mar 24, 2021

No description provided.

@ghost
Copy link

ghost commented Mar 24, 2021

CLA assistant check
All CLA requirements met.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
strategy:
matrix:
os: [windows, ubuntu]

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove double empty lines

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch!

@rwjblue
Copy link
Collaborator Author

rwjblue commented Mar 24, 2021

Currently failing due to #5 (moving to draft while we fix that).

@rwjblue rwjblue marked this pull request as draft March 24, 2021 21:38
@rwjblue rwjblue force-pushed the rwjblue/ci-setup branch 2 times, most recently from 0346525 to 826b247 Compare March 24, 2021 21:52
@jeffersonking
Copy link
Collaborator

jeffersonking commented Mar 24, 2021

What is the consideration between using macos vs ubuntu as the representative for UNIX?

I presume we have more macos usage than other UNIX?

@rwjblue
Copy link
Collaborator Author

rwjblue commented Mar 24, 2021

FWIW - CI is failing until we fix #5 (which @scalvert has in flight over in #6)

@rwjblue
Copy link
Collaborator Author

rwjblue commented Mar 24, 2021

@jeffersonking

What is the consideration between using macos vs ubuntu as the representative for UNIX?

I presume we have more macos usage than other UNIX?

Yes, you are totally correct - I would absolutely expect us to have more macOS users than Ubuntu users. Unfortunately, the macOS CI jobs are a bit finicky (slow to start, less overall capacity, transient failures, etc) in other repos whereas the Ubuntu ones seem to be pretty rock solid.

Additionally (I think I mentioned this inline in another comment), we don't really know if we will have platform specific code ATM. Even if we eventually do have a number of platform specific things, it is very likely to be the same between macOS and Ubuntu (essentially Windows is the one where most platform specific difference are super obvious).

@jeffersonking
Copy link
Collaborator

finicky

Thanks! That's valuable insight. I've been using macos on very small scales so never noticed. :-)

@scalvert
Copy link
Collaborator

@jeffersonking agree with what @rwjblue is saying, and that's been my experience too. The macos jobs would quite frequently fail for random reasons, which of course can be quite annoying in terms of test stability. Having to repeatedly re-run the jobs not only ended up being inconvenient, but it also degraded the confidence level of the stability of the test suites.

@scalvert
Copy link
Collaborator

@rwjblue needs a rebase with main.

@scalvert
Copy link
Collaborator

Probably good to mark this ready for review too, after the rebase.

Without this, `prettier` has issue with `\r\n` on windows (it expects
only `\n`).
@rwjblue rwjblue marked this pull request as ready for review March 24, 2021 23:24
@scalvert scalvert merged commit 7eaab30 into main Mar 24, 2021
@scalvert scalvert deleted the rwjblue/ci-setup branch March 24, 2021 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants