search

ExaWorks / SDK

ExaWorks SDK
11 stars 12 forks source link

Feature/gh ci #22

Closed andre-merzky closed 3 years ago

andre-merzky commented 3 years ago

This PR is used to test the use of docker based tests via GH actions. Please don't review just yet, it will take a while to stabilize... Thanks.

andre-merzky commented 3 years ago

Oh, it actually works :-P

So, this PR adds one CI workflow based on the RP dockerfile from #20 , runs that as GH action on any push to a PR (like this one), and on master, and then displays a badge in the top level [README] on that branch.

The docker build is fairly heavyweight - I am not sure how that works with githubs, i.e., if there are any limits we need to be aware of. We can (and should) probably optimize by uploading built base-containers to dockerhub and then to only incrementally on top of those. I think...

This PR fixes #8

dongahn commented 3 years ago

@andre-merzky: Thanks Andre. Is this ready for a review?

In the meantime, do you think you can squash some of the intermediate commits? Can we also use a bit more descriptions in the commit messages. Such will come in handy when we ever have to debug things. I understand we still need to put in general contribution guides and I may be asking things that have not been agreed upon yet though.

andre-merzky commented 3 years ago

@andre-merzky: Thanks Andre. Is this ready for a review?

Yes, indeed it is!

In the meantime, do you think you can squash some of the intermediate commits? Can we also use a bit more descriptions in the commit messages. Such will come in handy when we ever have to debug things. I understand we still need to put in general contribution guides and I may be asking things that have not been agreed upon yet though.

I realized already that you guys use a somewhat different commit / merge policy than we do. Not a problem, I don't mind to adjust and to see how that goes, but it probably will take a bit to get used to it :-) I put some questions about it in slack.

andre-merzky commented 3 years ago

Squash is done, commit message was expanded.

dongahn commented 3 years ago

@andre-merzky: a PR went in. Please rebase.

BTW, when PR #25 gets in, our process would be much more straightforward and transparent. @SteVwonder: we need to pull in Mergefyio soon so that merge conflicts will be automatically detects and if possible resolved. We don't want to manual manage that process.

andre-merzky commented 3 years ago

Please rebase

@dongahn : I did so. The branch alas had already merged from Master, so the changes no those show up in the diff also. Sorry for that. As you say, that will no happen once we adopt the policy...

dongahn commented 3 years ago

Just to be clear if I pushed the merge button, this won't override anything, correct?

andre-merzky commented 3 years ago

Just to be clear if I pushed the merge button, this won't override anything, correct?

correct