Skip to content

qualcomm ai100 docker support#11

Open
hpeliuhan wants to merge 2 commits into
waggle-sensor:mainfrom
hpeliuhan:qualcomm_ai100-docker-support
Open

qualcomm ai100 docker support#11
hpeliuhan wants to merge 2 commits into
waggle-sensor:mainfrom
hpeliuhan:qualcomm_ai100-docker-support

Conversation

@hpeliuhan

Copy link
Copy Markdown
Contributor

No description provided.

@seanshahkarami seanshahkarami left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hello! Just starting to look at this PR now.

I noticed that a number of script have their mode changed from 755 to 644. Was this a mistake?

I'm concerned that this will have some downstream effects on services which may rely on this.

@seanshahkarami

seanshahkarami commented Sep 6, 2023

Copy link
Copy Markdown
Member

When running ./build.sh -f -h I noticed that you runs this:

COPY qualcomm_drivers /qualcomm_drivers

Is there a step I should do before this? It doesn't seem like anything is mentioned in the docs about where to get this? Is it ok if it's empty when leaving out the -q flag?

@seanshahkarami

seanshahkarami commented Sep 6, 2023

Copy link
Copy Markdown
Member

Really minor detail - we should figure out a different way to select the build platform since many people will probably expect ./build -h to mean help instead of "build for HPE". 😄

@seanshahkarami

Copy link
Copy Markdown
Member

@hpeliuhan Just wanted to bring up the comments above again, specifically this one.

Hello! Just starting to look at this PR now.

I noticed that a number of script have their mode changed from 755 to 644. Was this a mistake?

I'm concerned that this will have some downstream effects on services which may rely on this.

Unfortunately, changing some of the key scripts to be non-executable will likely break things and I'd like to fix that before merging.

We can probably skip the other ones for now and fix that in a later PR.

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