Comments (6)
Ah yup, that explanation made it click for me. It's not the direct commit to a branch but the merge into main itself that gets checked.
So yeah, all of what applied before and also add the clause to run on both PR and commit which is possible in one unified file should do the trick
on:
push:
branches:
- main
pull_request:
We could also remove the master
branch filter as I don't see a reason where that would need to apply in this repo.
from elixir.
It looks like the checks are relaxed when committing directly to the repository, and some differences in the CI cache.
- pr-check.sh checks concepts and exercise order (on top of what ci-check.sh checks already)
- pr.ci for some reason computes the cache key twice in different steps
- ci uses a different cache key for
Retrieve Mix Dependencies Cache in the final step
, and skips redundant compute step
Otherwise the files are line for line save for some names (PR vs Main CI, on push vs on pr), and pr.sh
and ci.sh
are identical.
What do you think, the extra two concept/exercise checks could be merged into ci-check.sh
, and the cache values in ci.yml
seem fine?
Looking at the history, they were even more similar at the beginning. The cache key computation was the same, and the *-check.sh
files were identical. The calls to pr.sh
and ci.sh
were different initially, but were unified at some point.
I'm guessing there was a plan to have separate checks for maintainers and prs that never came to fruition. I don't see anything being lost if they are merged, and maintainers will receive extra checks when committing directly to a branch.
from elixir.
diff --git a/.github/workflows/ci.yml b/.github/workflows/pr.ci.yml
index e0990168..dbf4039c 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/pr.ci.yml
@@ -1,15 +1,12 @@
# This workflow will do a clean install of the dependencies and run tests across different versions
#
# Requires scripts:
-# - bin/ci.sh
-# - bin/ci-check.sh
+# - bin/pr-check.sh
+# - bin/pr.sh
-name: elixir / main ci
+name: elixir / pr
-on:
- push:
- branches: [master, main]
- workflow_dispatch:
+on: pull_request
jobs:
precheck:
@@ -20,7 +17,7 @@ jobs:
otp: [26.0]
steps:
- - name: Checkout code
+ - name: Checkout PR
uses: actions/checkout@v3
- name: Use Elixir
@@ -66,8 +63,8 @@ jobs:
mkdir -p priv/plts
mix dialyzer --plt
- - name: Run Prechecks
- run: bin/ci-check.sh
+ - name: Run exercism/elixir ci pre-check (stub files, config integrity)
+ run: bin/pr-check.sh
ci:
runs-on: ubuntu-20.04
@@ -98,12 +95,22 @@ jobs:
otp-version: ${{matrix.otp}}
elixir-version: ${{matrix.elixir}}
+ - name: Set cache key
+ id: set_cache_key
+ run: |
+ erl -eval '{ok, Version} = file:read_file(filename:join([code:root_dir(), "releases", erlang:system_info(otp_release), "OTP_VERSION"])), io:fwrite(Version), halt().' -noshell > ERLANG_VERSION
+ cat ERLANG_VERSION
+ elixir --version | tail -n 1 > ELIXIR_VERSION
+ cat ELIXIR_VERSION
+ cache_key="os-${{ runner.os }}-erlang-$( sha256sum ERLANG_VERSION | cut -d ' ' -f 1 )-elixir-$( sha256sum ELIXIR_VERSION | cut -d ' ' -f 1 )-mix-lock-${{ hashFiles(format('{0}{1}', github.workspace, '/mix.lock')) }}"
+ echo "::set-output name=cache_key::$cache_key"
+
- name: Retrieve Mix Dependencies Cache
uses: actions/cache@v3
id: mix-cache # id to use in retrieve action
with:
path: deps
- key: ${{ runner.os }}-${{ matrix.otp }}-${{ matrix.elixir }}-mix-${{ hashFiles(format('{0}{1}', github.workspace, '/mix.lock')) }}-v2
+ key: mix-${{ steps.set_cache_key.outputs.cache_key }}-v1
- name: Install Mix Dependencies
if: steps.mix-cache.outputs.cache-hit != 'true'
@@ -112,5 +119,5 @@ jobs:
- name: Build Project
run: mix
- - name: Run Checks
- run: bin/ci.sh
+ - name: Run exercism/elixir ci (runs tests)
+ run: bin/pr.sh
from elixir.
Hmm. That is a very small difference between the two workflows. I think they just should be merged.
Pushing directly to main is not allowed, so every change will go through the GitHub PR process. Still, I think both workflows need to be the same.
If I understand correctly, the PR workflow will run when a PR is open and then on every new commit to the PR. The workflow runs against the code in the PR. Then the PR waits for approval and merging. In that time, changes to main might happen that do not trigger git conflicts (do not force you to edit the PR), but will cause the workflow to fail. For example - two people adding two exercises in two different PRs with the same id. So I think for technical reasons, the CI workflow (run on commits to main) only needs to run checks that can trigger errors due to parallel changes like that (not causing conflicts). However, in practice, it is just much more sane to always run exactly the same code quality checks in both cases. We might lose some time waiting longer for the workflows to finish, but we won't need to deal with forgetting to make a change in one of the workflows but not the other 🤔
from elixir.
Oh okay, so basically the main-ci workflow runs when you commit to a branch other than main right? So that would be checked before being PR'd into main. Isn't that a redundant check?
from elixir.
I'm not sure I understand the question correctly. If you're asking about the hypothetical ideal solution, then we want a single CI workflow that runs on every commit on every PR and on every commit to main. This is necessary to ensure all PRs pass the checks, and that main still passes all the checks after merging two or more PRs in parallel that passed the checks separately, but might still cause a failure when applied together.
"Running on every PR" basically means running on every commit on a branch that has a PR open, as far as I understand.
from elixir.
Related Issues (20)
- Top Secret could use another test HOT 2
- [Dancing Dots] "defines a __using__ macro" test is flakey in certain scenarios HOT 2
- [log-level] Misused trailing question marks HOT 1
- Set up a dead link checker
- Update all Elixir Exercism repos to use Elixir 1.15 and OTP 26 HOT 8
- Adjust all concepts to use charlist sigils
- test_exercises.sh print fails when test output includes `%`
- Lasagna exercise HOT 2
- Phone Number exercise: difference between tests in exercise folder vs online Test runner HOT 2
- Implement new practice exercise: bottle-song
- Too forgiving test in exercises/concept/german-sysadmin HOT 1
- Mistakes in `bottle-song` example solution typespecs HOT 3
- [RPG Character Sheet] Misleading the 5th question HOT 4
- Building a training set of tags for elixir HOT 22
- Run on Elixir 1.16
- Leap approaches for 48in24
- 48in24 Approaches for Raindrops HOT 2
- Darts exercise hint for sqrt workaround doesn't seem to work HOT 7
- Typo In Take-A-Number Deluxe Step 6 HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from elixir.