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

Fixes to prepare-base.sh for the merged wifi setup #1242

Merged
merged 3 commits into from
Aug 2, 2018

Conversation

dhylands
Copy link
Contributor

@dhylands dhylands commented Aug 1, 2018

This should fix #1241

@ghost ghost assigned dhylands Aug 1, 2018
@ghost ghost added the review label Aug 1, 2018
@dhylands dhylands requested a review from hobinjk August 1, 2018 23:23
@codecov-io
Copy link

codecov-io commented Aug 1, 2018

Codecov Report

Merging #1242 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1242      +/-   ##
==========================================
+ Coverage   75.33%   75.35%   +0.01%     
==========================================
  Files         123      123              
  Lines        5916     5916              
  Branches      824      824              
==========================================
+ Hits         4457     4458       +1     
+ Misses       1287     1286       -1     
  Partials      172      172
Impacted Files Coverage Δ
src/controllers/things_controller.js 79.82% <0%> (+0.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7618a3...21a02b3. Read the comment docs.

Copy link
Contributor

@hobinjk hobinjk left a comment

Choose a reason for hiding this comment

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

LGTM with the existing config files in image/config removed. The inclusion of the systemd service file is also unnecessary. I don't understand why everything needs to be copied into the home directory for it to work but as long as it does work I'm okay with it

@dhylands
Copy link
Contributor Author

dhylands commented Aug 2, 2018

The make-prep.sh script copies a few files (prepare-base.sh and prepare-base-root.sh) to the home directory. Most of the files the scripts needs are actually embedded inside the script itself as a "here" document.

At this stage, we're setting up to build the base image, so the gateway repository hasn't been checked out or anything.

I'd probably update the scripts to use "here" documents in the future. See this for an example:
https://github.com/mozilla-iot/gateway/blob/6724d401cee3ca278b5f448c9d5adcfccd1910b3/image/prepare-base.sh#L79-L101

@dhylands dhylands merged commit 287d1ed into WebThingsIO:master Aug 2, 2018
@ghost ghost removed the review label Aug 2, 2018
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.

Error running prepare-base.sh
3 participants