-
Notifications
You must be signed in to change notification settings - Fork 228
Update circleCI scripts and image #1156
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
Conversation
1a705fe to
1d02b03
Compare
barendgehrels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not an expert here.
I welcome improvements so I approve this.
|
Thanks for this update! Btw, I'd be ok with using the default gcc version. It's probably more commonly used (I assume it's ubuntu 20.04) and requires to do less manual steps which have tendency to fail. FYI, currently the coverage step is failing due to some issues with package repositories so this change could fix it, maybe. Have you tested whether or not it works with your fork by pushing develop or master branch and checking at CircleCI? |
93e09bf to
8f55833
Compare
8f55833 to
52d2f7e
Compare
|
Hi @awulkiew thanks for the review. I revised my PR by adding a more straightforward solution, i.e. manually adding the missing tests. The previous approach by adding a closing job was end up too complicated (since I had to add also special cases to the |
|
There is also this related PR #1154 where I need your feedback. |
|
Thanks, I'm ok with merging it. Regarding testing in your fork, AFAIR you have to push them to your develop branch because otherwise minimal test is tested. You can also modify the script temporarily to always run it for all branches. Unless I'm mistaken. |
52d2f7e to
202f04c
Compare
202f04c to
8ab24e6
Compare
Following the discussion here this PR proposes:
a closing job to circleCI to covermissing tests.