Conversation
duhruh
commented
Jul 11, 2019

|
Please sign your commits following these rules: $ git clone -b "feature/web-page" git@github.com:duhruh/docker-install.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
seemethere
left a comment
There was a problem hiding this comment.
Have a bit of an issue the way you defined the make targets, but everything looks good to me.
No idea why you're getting all the shellcheck issues though.
Seems as though adding the html makes shellcheck unhappy.
Makefile
Outdated
| mkdir -p build | ||
|
|
||
| CLEAN += docker.sh | ||
| docker.sh: |
There was a problem hiding this comment.
This should actually just be build/docker.sh
And then the rule could be defined as:
.PHONY: build
build: build/docker.sh
build/docker.sh: install.sh index.html
mkdir -p build
@HTML='$(shell cat index.html)' envsubst < $< >> $@Seems a bit convoluted to do it the way you're doing it here.
|
A recent change to shellcheck seems to have made SC2034 a little more strict. https://github.com/koalaman/shellcheck/releases/tag/v0.6.0 released Dec 3, 2018 but the |
|
I also believe the |
6ba1e38 to
369867a
Compare
Signed-off-by: David Rivera <david.rivera@docker.com> Signed-off-by: Eli Uriegas <eli.uriegas@docker.com>
369867a to
d6635a8
Compare
| <meta name="apple-mobile-web-app-status-bar-style" content="black" /> | ||
| <meta name="HandheldFriendly" content="true" /> | ||
| <meta name="MobileOptimized" content="320" /> | ||
| <link href="/david.rivera/assets/style.css" rel="stylesheet" type="text/css"> |
There was a problem hiding this comment.
you should probably change this to a different href
| @@ -1,4 +1,9 @@ | |||
| #!/bin/sh | |||
|
|
|||
| cat >/dev/null <<\EOF | |||
There was a problem hiding this comment.
this is for shellcheck purposes
|
Hmm, actually I like to have the code shown in browser. Easiest way to quickly search through code from desktop. One can copy&paste or "save to file" as well. Not sure about the benefit of having a website instead which just shows the same command line that you very likely got the URL itself from? 😉 |