Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Add tests for every plateform + clippy #116

Merged
merged 3 commits into from
Apr 27, 2021
Merged

Add tests for every plateform + clippy #116

merged 3 commits into from
Apr 27, 2021

Conversation

curquiza
Copy link
Member

@curquiza curquiza commented Mar 22, 2021 •
edited
Loading

No description provided.

@curquiza curquiza force-pushed the all-plateforms-tests branch from ffe8576 to c961ff5 Compare March 22, 2021 18:19
@curquiza curquiza linked an issue Mar 22, 2021 that may be closed by this pull request
Test transplant on other platforms #95
Closed
@curquiza
Copy link
Member Author

Does the old MeiliSearch test on Windows?

@curquiza curquiza requested a review from MarinPostma March 22, 2021 18:25
@MarinPostma
Copy link
Contributor

Does the old MeiliSearch test on Windows?

I thought we did, but apparently not?

@curquiza
Copy link
Member Author

We indeed don't: https://github.com/meilisearch/MeiliSearch/blob/master/.github/workflows/test.yml
https://github.com/meilisearch/MeiliSearch/search?q=windows

I've just released my tests does not pass, I shouldn't have requested you for a reviewer still it's not fixed!
I'll try to add test for Windows as well!

@curquiza curquiza removed the request for review from MarinPostma March 29, 2021 08:47
@curquiza curquiza force-pushed the all-plateforms-tests branch from 7bc0c97 to b27860d Compare March 29, 2021 08:47
@curquiza curquiza changed the title Add tests on every platform Add tests to every platform Apr 7, 2021
@curquiza curquiza force-pushed the all-plateforms-tests branch from b27860d to d913a04 Compare April 7, 2021 18:38
@curquiza curquiza force-pushed the all-plateforms-tests branch 3 times, most recently from 8078ea7 to c77b277 Compare April 21, 2021 16:48
@curquiza
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Apr 21, 2021
@bors
Try #116:
e35c583
@curquiza
Copy link
Member Author

Cannot add tests on Windows since we get the "There is not enough space on the disk." errror
https://github.com/meilisearch/transplant/runs/2402295402

@bors
Copy link
Contributor

bors bot commented Apr 21, 2021

try

Build failed:

@curquiza curquiza force-pushed the all-plateforms-tests branch from c77b277 to 55bbbc7 Compare April 21, 2021 16:55
@curquiza
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Apr 21, 2021
@bors
Try #116:
801a40c
@bors
Copy link
Contributor

bors bot commented Apr 21, 2021

try

Build succeeded:

@curquiza curquiza requested a review from MarinPostma April 21, 2021 18:30
@curquiza curquiza marked this pull request as ready for review April 22, 2021 10:07
@curquiza curquiza force-pushed the all-plateforms-tests branch from 9adb26f to ebd897e Compare April 22, 2021 10:09
@curquiza curquiza changed the title Add tests to every platform Add tests for every plateform + clippy Apr 22, 2021
@curquiza
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Apr 22, 2021
@bors
Try #116:
af0e69a
@bors
Copy link
Contributor

bors bot commented Apr 22, 2021

try

Build succeeded:

MarinPostma
MarinPostma suggested changes Apr 27, 2021
View reviewed changes
meilisearch-http/src/index_controller/update_actor/update_store.rs Outdated
@@ -13,16 +13,16 @@ use uuid::Uuid;
use crate::helpers::EnvSizer;
use crate::index_controller::updates::*;

type BEU64 = heed::zerocopy::U64<heed::byteorder::BE>;
type Beu64 = heed::zerocopy::U64<heed::byteorder::BE>;
Copy link
Contributor

Choose a reason for hiding this comment

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

This name should remain like this. add #[allow(non_camel_case)] on top of it should fix it

Copy link
Member Author

Choose a reason for hiding this comment

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

Not pb, I change this. By curiosity, could you quickly explain why? 🙂

Copy link
Contributor

@MarinPostma MarinPostma Apr 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

this is an acronym for Big Endian U64, so it makes more sense that it is all uppercased
This is also how we refer to this type in milli, and meilisearch :)

@curquiza curquiza force-pushed the all-plateforms-tests branch from 7dd22e5 to 0c41adf Compare April 27, 2021 10:43
MarinPostma
MarinPostma approved these changes Apr 27, 2021
View reviewed changes
Copy link
Contributor

@MarinPostma MarinPostma left a comment

Choose a reason for hiding this comment

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

lgtm

bors merge

@bors
Copy link
Contributor

bors bot commented Apr 27, 2021

Build succeeded:

@bors bors bot merged commit 7055384 into main Apr 27, 2021
@bors bors bot deleted the all-plateforms-tests branch April 27, 2021 11:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MarinPostma MarinPostma MarinPostma approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test transplant on other platforms
2 participants
@curquiza @MarinPostma