Combine workflows to pr.yml #313
Conversation
nirinchev
changed the title
nirinchev
force-pushed
the
ni/combine-workflows
branch
from
b5fc378
to
77be6ef
Compare
Pull Request Test Coverage Report for Build 2070440925
💛 - Coveralls |
| @@ -51,15 +53,14 @@ jobs: | |||
| submodules: 'recursive' | |||
| - name: Setup Ninja | |||
| uses: seanmiddleditch/gha-setup-ninja@master | |||
| - name: Build Realm Dart for Linux | |||
| - name: Build Realm for Linux | |||
There was a problem hiding this comment.
I'm okay with this change, but the library being build is librealm_dart.so. I have wondered, if it would be better to build two libs, ie. also a librealm.so, so that the later can be shared across SDKs, with the Dart VM specific parts only, being in librealm_dart.so. Anyway - that is not for this PR, just something to think about.
There was a problem hiding this comment.
This is the name of the library used by both flutter and dart - at least looking at the latest release, all libraries except for iOS are librealm_dart. I'm fine with renaming it to librealm, but I agree it should be done as a separate PR as it will likely be more involved than what I wanted to do in this one.
There was a problem hiding this comment.
Why are we thinking renaming the library. I think we are totally sidetracking here. It's fine to rename the step.
There was a problem hiding this comment.
Not sure I understand - why would we rename the step? This is building the native Realm library for Linux. It's used by both dart and flutter, which is why I think it's incorrect to call it "Build Realm Dart".
There was a problem hiding this comment.
Also, I don't think it's particularly important to rename the library and in any case, not right now.
| if(CMAKE_SYSTEM_NAME STREQUAL "Windows") | ||
| string(APPEND OUTPUT_DIR "/windows") | ||
| elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux") | ||
| elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux") |
|
The CI failures are related to a botched deployment of Atlas on cloud-dev. The team is investigating but they're not a merge stopper. |
Based on #309, will rebase once merged.