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

Multi-platform & multi-version CI #176

Merged
merged 4 commits into from
Jan 25, 2021
Merged

Multi-platform & multi-version CI #176

merged 4 commits into from
Jan 25, 2021

Conversation

vaind
Copy link
Contributor

@vaind vaind commented Jan 25, 2021

@vaind vaind changed the title Multiplatform CI Multi-platform & multi-version CI Jan 25, 2021
@vaind vaind force-pushed the multiplatform-ci branch 19 times, most recently from e9305c3 to 16e5905 Compare January 25, 2021 12:11
Copy link
Member

@greenrobot-team greenrobot-team left a comment

Choose a reason for hiding this comment

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

LGTM, just some questions.

DynamicLibrary lib;
if (Platform.isWindows) {
// DynamicLibrary.process() is not available on Windows
lib = DynamicLibrary.open('msvcr100.dll');
Copy link
Member

Choose a reason for hiding this comment

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

Where is this workaround coming from? Appears to be a C library. Does this require installation of Microsoft Visual C++ 2015 Redistributable (x64) like mentioned at https://docs.objectbox.io/java-desktop-apps#native-libraries? Should this be noted in a README?

The GitHub Windows instance has a bunch of versions installed, so I guess most are fine. https://github.com/actions/virtual-environments/blob/win19/20201210.0/images/win/Windows2019-Readme.md#microsoft-visual-c

Copy link
Contributor Author

@vaind vaind Jan 25, 2021

Choose a reason for hiding this comment

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

Where is this workaround coming from?

Nowhere in particular, I've just searched what libary defines memset on windows. Turns out it's mscvrt.dll or mscvr100.dll. Yes it may be fragile even though the library is pretty common, some users might not have it... I'll add another fallback layer

Copy link
Member

Choose a reason for hiding this comment

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

Maybe add this in a comment then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, also added a comment

lib/src/query/query.dart Show resolved Hide resolved
pubspec.yaml Show resolved Hide resolved
tool/pub.sh Show resolved Hide resolved
@vaind vaind merged commit d3045ee into main Jan 25, 2021
@vaind vaind deleted the multiplatform-ci branch January 25, 2021 15:11
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.

CI - test all supported dart/flutter versions CI on macOS and Windows
2 participants