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

WIP [ENH] Ci linux using cl scripts #820

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

juangpc
Copy link
Collaborator

@juangpc juangpc commented Aug 3, 2021

The objective of this PR is to, soon, be able to run, with exactly the same script, jobs locally and in the GitHub actions. There is still a bit of work to do.

  • Understand completely what do all the different GitHub workflows do.
  • Create a single multiplatform script for each workflow.
  • I'd like to discuss if we should just use python for all these and forgive us all of using bash scripting.
  • Standardize how to treat input arguments for all scripts. Source all the functions into one single script.
  • validate results.
  • ready to merge.
  • Harmonize the use of windows versions (I see sometimes 2016 and some other times 2019).

@juangpc juangpc changed the title [WP][ENH] Ci linux using cl scripts WP [ENH] Ci linux using cl scripts Aug 17, 2021
@juangpc juangpc changed the title WP [ENH] Ci linux using cl scripts WIP [ENH] Ci linux using cl scripts Sep 14, 2021
make module-qtbase module-qtsvg module-qtcharts module-qt3d -j4
make install -j4
cd ..
cd mne-cpp
Copy link
Member

Choose a reason for hiding this comment

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

Why would you recompile qt from scratch every time we trigger a CI job?

:;# For more information you can visit: https://github.com/juangpc/multiplatform_bash_cmd
:;#

:<<BATCH
Copy link
Member

Choose a reason for hiding this comment

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

this file is very hard to read. I would split it up for the different platforms build_mac.sh.bat, build_win-sh.bat and build_linux.sh.bat. Same goes for the deployment script.

@LorenzE
Copy link
Member

LorenzE commented Oct 9, 2021

@juangpc Is the goal of the PR to introduce a build script so that we can more easily build the project locally exactly just as we do in the CI? This would probably be a good idea since it will make the CI workflow yaml files easier to read.

I am just a bit confused because you mentioned "jobs" in the PR description. We should not try to replace every job in all the workflows by our own scripts. Workflows can house jobs, jobs are composed out of multiple steps, which again can be represented by Github actions. There are a lot of great Github actions available and we should not try to replace them with scripts written by us. Just take the Install Qt action as an example, rewriting this on our own is not a good idea. Not sure if this was your intention, but if it was we should discuss.

@juangpc
Copy link
Collaborator Author

juangpc commented Oct 9, 2021

I think I understand what you mean, and in principle I agree with you. But, let's take your example: i already have a script that builds from qt from sources . I had to do it because I needed to understand why codecov script was not working. This was a few weeks ago, you probably remember. That time the task of fixing codecov figures, was,in my opinion unecesarily cumbersome due not having the script ready.

It is a bit of a philosophy approach. Do you want a ci that replicates your duties automatically in the cloud or do you want it to independent process that checks your code.

I'm more inclined to make a ci something that you work with, everyday, locally. Compilation steps, testing, code coverage, I'd like to know locally and then replicate automatically in the cloud before merging.

So the next question is. How do you do that with JS GitHub actions?

So yes. Let's discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants