Skip to content

Conversation

rafie
Copy link
Contributor

@rafie rafie commented Aug 27, 2019

No description provided.

@rafie rafie requested a review from lantiga August 28, 2019 08:19
@lantiga
Copy link
Contributor

lantiga commented Sep 1, 2019

Hi @rafie, I tested this on my system and it worked flawlessly.

I also tested an out-of-source build, in a RedisAI-build directory parallel to the RedisAI source directory. It worked without issues, the only nit is that the install-cpu directory was created inside the source directory.

This was kind of unexpected, as I expected the install directory to be outside the source dir, but in fact placing it in the RedisAI-build directory is quite confusing too, since we'd have redisai.so in RedisAI-build and another redisai.so in RedisAI-build/install-cpu.

The ideal would be to have a RedisAI-install parallel to RedisAI-build, but we can't just create directories outside the build root.

So, long story short, we should keep it in the source tree for now :-D

I'm good to merge, but maybe we want to take this opportunity to rearrange directories as we discussed on Slack?

lantiga
lantiga previously approved these changes Sep 1, 2019
@lantiga lantiga merged commit bfbc376 into master Sep 1, 2019
@rafie rafie deleted the rafi-arm2 branch September 2, 2019 10:50
lantiga pushed a commit that referenced this pull request May 6, 2020
* ARM support and bin/os-arch-variant scheme

* Build: fixes #1

* Build: fixes #2

* Build: fixes #3

* Build: fixes #4

* Build fixes #5

* CircleCI config.yml refectoring

* Build fixes #6

* Build fixes #7

* Build fixes #8

* Build fixes #9

* Build fixes #10

* Build fixes #11

* Build fixes #12

* Build fixes #13

* Build fixes #14

* Build fixes #15

* Build fixes #16

* Build fixes #17

* Build fixes #18

* Build fixes #19

* Build fixes #20

* Build fixes #21

* Build fixes #22

* Build fixes #23

* Build fixes #24

* Build fixes #25

* Build fixes #26

* Filesystem restructuring

* Pack fixes + docker goal in makefile
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.

2 participants