dylanirlbeck / tailwind-ppx

A Reason/OCaml Pre-Processor eXtension (PPX) that validates your Tailwind classes at compile-time.
MIT License
152 stars 15 forks source link

CI: improve workflow #100

Closed tatchi closed 4 years ago

tatchi commented 4 years ago

This gets rid of the bash scripts which block #93 and generally improve the whole CI process (I think). This is mostly based on redemon. Here is a general overview of the steps:

1. build_and_test

2. prepare-publish

3. test-platform

4. publish

Note that for now, the test-platform fails because the --help command returns an exit code 2 (while it works correctly). I tested it with #93 and it should be fixed (I guess due to one of the dependencies update)

tatchi commented 4 years ago

Do we want to remove scripts/release-make-skeleton.js? It seems that you new bundle-release.js file effectively replaces it.

I removed it. Should we get rid of the release-postinstall.js one we well ?

Finally, I see some of the test and install workflows are failing in CI. Can you take a look at this? I assume I'll be changing around which ones are required for pull requests, but let's just make sure everything is passing before merging in.

Yes, that's normal. As explained, the --help command currently returns with an exit code 2 on Linux and macOS. This makes the CI step to fail. It should be fixed with #93. So once this one is merged, I will rebase #93 and everything should be ok (hopefully)

tatchi commented 4 years ago

@dylanirlbeck I rebased on master now that #83 is merged and CI is green :)

tatchi commented 4 years ago
  1. Is there a way you can test that the use-tailwind-ppx script works on each platform? In the README I listed the different ways to call the script, so perhaps we could throw in a check there? I just want to make sure nothing is broken with the new flow.

I added that to the CI config. A new BS project is bootstrapped where the use-tailwind-ppx command is run. Nothing is actually happening since there's nothing to convert but at least we make sure that the command is available on each platform.

  1. We do not want to remove the release-postinstall.js script, because that's what gets copied into the release zip folder and is run when a user installs the NPM package.

I think we can remove it. I use another one for the CI stuff now (taken from the hello-reason template). So except if you use it elsewhere, it should be ok to delete it.

  1. After we merge this, which GitHub actions checks should be required on PRs? It looks like every check except the (only on release) Publish one, right?

Yes indeed :) The publish one should get triggered when you create a tag.