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

docs: image base reference #545

Merged
merged 1 commit into from
Mar 9, 2017
Merged

Conversation

vbatts
Copy link
Member

@vbatts vbatts commented Jan 31, 2017

Fixes #247
Fixes #435

this makes an ephemeral header that gets included into the html doc that
bases the images path to pull from the same tag on github.

CC @RobDolinMS @wking

Signed-off-by: Vincent Batts vbatts@hashbangbash.com

func main() {
version := specs.Version
if strings.HasSuffix(specs.Version, "-dev") {
version = "master"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just keep the *-dev version?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just keep the *-dev version?

Ah, because it won't work for <base>. Maybe use specs.Version directly in <title>, and then use the HEAD commit hash in <base> when specs.Version ends with -dev.

Copy link
Member Author

Choose a reason for hiding this comment

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

lemme take a stab at that.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated.

Makefile Outdated
@@ -65,16 +65,19 @@ $(OUTPUT_DIRNAME)/$(DOC_FILENAME).pdf: $(DOC_FILES) $(FIGURE_FILES)
else
$(OUTPUT_DIRNAME)/$(DOC_FILENAME).pdf: $(DOC_FILES) $(FIGURE_FILES)
@mkdir -p $(OUTPUT_DIRNAME)/ && \
$(PANDOC) -f markdown_github -t latex -o $(PANDOC_DST)$@ $(patsubst %,$(PANDOC_SRC)%,$(DOC_FILES))
$(PANDOC) -f markdown_github -t latex -o $(PANDOC_DST)$@ $(patsubst %,$(PANDOC_SRC)%,$(DOC_FILES)) && \
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for && chaining. Make will die if any individual command fails.

Copy link
Member Author

Choose a reason for hiding this comment

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

i don't recall that to be the case. It's not like there is an implied set -e inside the target

Copy link
Contributor

Choose a reason for hiding this comment

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

i don't recall that to be the case. It's not like there is an implied set -e inside the target

$ cat Makefile
foo:
        false
        echo "still going?"
$ make
false
Makefile:2: recipe for target 'foo' failed
make: *** [foo] Error 1

If that's not convincing enough, I can try to hunt down some docs for you, just let me know.

Copy link
Member Author

Choose a reason for hiding this comment

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

heh. fair. my memory fails me on the need for painful amounts of new-line escapes for long one-liners. No biggie.

Makefile Outdated
@mkdir -p $(OUTPUT_DIRNAME)/ && \
cp -ap img/ $(shell pwd)/$(OUTPUT_DIRNAME)/&& \
$(PANDOC) -f markdown_github -t html5 -o $(PANDOC_DST)$@ $(patsubst %,$(PANDOC_SRC)%,$(DOC_FILES))
$(PANDOC) -f markdown_github -t html5 -H $(patsubst %,$(PANDOC_SRC)%,header.html) --standalone -o $(PANDOC_DST)$@ $(patsubst %,$(PANDOC_SRC)%,$(DOC_FILES)) && \
Copy link
Contributor

Choose a reason for hiding this comment

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

$(PANDOC_SRC)header.html should work fine, and it reads more clearly (to me, anyway).

Copy link
Member Author

Choose a reason for hiding this comment

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

fair. copy pasta. fixed

@vbatts vbatts force-pushed the html_img branch 3 times, most recently from 5cf770b to cec7700 Compare February 1, 2017 22:37
@vbatts
Copy link
Member Author

vbatts commented Feb 1, 2017

updated. PTAL.

@wking
Copy link
Contributor

wking commented Feb 2, 2017 via email

@vbatts
Copy link
Member Author

vbatts commented Feb 2, 2017

oh oh. You're saying to get the commit and reference the precise remote head commit. hrm.

@vbatts
Copy link
Member Author

vbatts commented Feb 2, 2017

fixed. Now a -dev version is pin the <base> at that HEAD commit that the docs were built from (which pre-supposes that the commit exists on github.com, but so it goes).

@wking
Copy link
Contributor

wking commented Feb 2, 2017 via email

@philips
Copy link
Contributor

philips commented Feb 18, 2017

LGTM

Approved with PullApprove

@jonboulle
Copy link
Contributor

needs rebase

@stevvooe
Copy link
Contributor

stevvooe commented Mar 7, 2017

@vbatts Please rebase!

@vbatts
Copy link
Member Author

vbatts commented Mar 8, 2017

rebased, but will need to rebase again on once master is fixed ...

Fixes opencontainers#247

this makes an ephemeral header that gets included into the html doc that
bases the images path to pull from the same tag on github.

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
@vbatts
Copy link
Member Author

vbatts commented Mar 8, 2017

rebased

@stevvooe
Copy link
Contributor

stevvooe commented Mar 8, 2017

LGTM

Approved with PullApprove

@jonboulle
Copy link
Contributor

jonboulle commented Mar 9, 2017

LGTM, lol, whatever

Approved with PullApprove

@jonboulle jonboulle merged commit 5d925ee into opencontainers:master Mar 9, 2017
@vbatts
Copy link
Member Author

vbatts commented Mar 9, 2017

@jonboulle you mad

@vbatts vbatts deleted the html_img branch March 9, 2017 14:01
@jonboulle
Copy link
Contributor

@vbatts dat html/pdf generation
wut

@vbatts vbatts mentioned this pull request May 19, 2017
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.

5 participants