refactor CI script #34
Conversation
| @@ -22,10 +22,31 @@ | |||
| subdirectory. | |||
|
|
|||
|
|
|||
| ## Configuring tests | |||
There was a problem hiding this comment.
I would rather always have the config.yml (possibly better name menu.yml) and nothing left to default. It simplifies the logic downstream. Upstream (i.e. .travisy.yml and .appveyor.yml) we can install all dependencies without logical switches on environment variables.
There was a problem hiding this comment.
No problem to have this mandatory but would prefer to have this outside this PR. However I will rename to menu.yml and append to this PR.
There was a problem hiding this comment.
Mandatory menu.yml followed up in #36.
There was a problem hiding this comment.
Fine by me. One step at a time.
| Example: | ||
| ```yaml | ||
| # environment variables to be set | ||
| env: |
There was a problem hiding this comment.
I think the best practice for compilers is to pass CMAKE_<LANG>_COMPILER with -D rather than set environment variables.
There was a problem hiding this comment.
OK - I will update the example.
.scripts/ci_configure_build_test.py
Outdated
| configure = run_command(['cmake', '-H.', | ||
| '-Bbuild', '-G' + generator]) | ||
| [print(x, end="") for x in configure] | ||
| sys.stdout.write('{}/{}\n'.format(recipe, example)) |
There was a problem hiding this comment.
Can we parse also the README.md one directory up to get the name of the recipe and print it out?
There was a problem hiding this comment.
Good idea, will append this to this PR.
|
Yeah also Travis did not do anything previously. We need to fix things. Need to append few changes and will ping once it is ready for another review and merge. |
|
Voila - ready for review. |
|
Oh - one commit message says "rename menu.yml to config.yml" although it does the exact opposite. Will edit and re-push. |
bast
force-pushed
the
radovan/refactor-ci-script
branch
from
25ed2bf
to
585dc6c
Compare
|
I think AppVeyor doesn't like the regex... Can be fixed in a later PR. Great work so far 😉 I'll merge now and look at the failures tomorrow. |
Probably breaks stuff on Appveyor but it gives now less output, more informative output, and the possibility to provide per-recipe environments. I recommend that we merge (but still please review it) and then we iron out issues. It is still not perfect but I believe a step towards the better.