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

Migrate website to Hugo #2342

Merged
merged 1 commit into from
May 16, 2018
Merged

Migrate website to Hugo #2342

merged 1 commit into from
May 16, 2018

Conversation

lucperkins
Copy link
Contributor

@lucperkins lucperkins commented May 14, 2018

This PR migrates the current website from Jekyll to Hugo and updates both the documentation for running the site locally and the build process.

Fixes issue #2342.

@dmcgowan I may need a bit of guidance with this in ensuring a graceful transition.

@dmcgowan
Copy link
Member

Thanks! Let me see if I can get netlify updated so we can compare

@lucperkins
Copy link
Contributor Author

@dmcgowan I can add a Netlify configuration that should solve the current issue.

@@ -0,0 +1,13 @@
[build]
publish = "public"
command = "make build"
Copy link
Member

Choose a reason for hiding this comment

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

I think the source directory has to be set, the command gets run from root. Not sure if this file also has to be in the root.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like the Netlify config allows for a non-root directory, which seems better to me than having the website config in the root.

@lucperkins
Copy link
Contributor Author

@dmcgowan It looks like Netlify is still trying to build the site as a Jekyll site. There may be necessary configuration changes in the Netlify web UI. Could you check?

@dmcgowan
Copy link
Member

Not sure how to get it to respect the toml file, updated to hugo and rebuilt.

4:01:41 PM: Error: Error building site: Failed to read data from features.yaml/features.yaml: yaml: unmarshal errors:
4:01:41 PM:   line 1: cannot unmarshal !!seq into map[string]interface {}

@@ -0,0 +1,7 @@
- OCI Image Spec support
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a map expected here cannot unmarshal !!seq into map[string]interface {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird. That works just fine locally. I'll look into it.

Copy link
Member

Choose a reason for hiding this comment

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

Works locally for me as well, it appears to be running the command I set on the project rather than from the toml. I don't know what version of hugo it is running.

Copy link
Member

Choose a reason for hiding this comment

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

My bad, I needed to set that environment variable, build working now

@dmcgowan
Copy link
Member

Works, site looks good. Can you sign/squash some of the commits and remove the netlify.toml file and commits. Since it is not being used I would prefer not to have it until we figure it out.

@dmcgowan
Copy link
Member

The only thing missing in the new version is the text right under the logo. Compare https://5afa1bdedd6a546d33baef24--musing-einstein-7cc486.netlify.com/ vs https://containerd.io/

@lucperkins
Copy link
Contributor Author

My bad. Fixed. I can squash the commits later this evening.

@caniszczyk
Copy link
Contributor

also make sure the commit is signed off... git commit -s

@dmcgowan I assume we're still checking for it but I don't see the DCO bot

@dmcgowan
Copy link
Member

@caniszczyk Travis is checking, which is causing the failure. We currently don't use a separate check and just have it as part of the Travis run.

@lucperkins sign off then looks good. Could you also squash some of the commits, makes signing off much easier and would prefer to remove commits which add then remove files in the same PR.

@lucperkins
Copy link
Contributor Author

@dmcgowan I didn't see your note and squashed everything into a single commit. I hope that's okay!

docs/README.md Outdated
To build the site locally (using Docker):

```bash
$ docker run -it -v $(pwd):/src -p "1313:1313" jojomi/hugo
Copy link
Member

Choose a reason for hiding this comment

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

Is -it required?
Also, probably --rm is expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that there's really a use case for just building the site using Docker, so I removed these instructions and left only the instructions below for "watch" mode.

@ehazlett
Copy link
Member

@lucperkins site looks good! There are a couple validation checks still failing.

DCO check is complaining of some trailing whitespace:

docs/content/docs/dockercon-summit.md:8: trailing whitespace.
+This year at Dockercon US 2017 we will be having a containerd Summit on Thursday morning the week of the conference.
docs/content/docs/dockercon-summit.md:10: trailing whitespace.
+We are going to change the format slightly compared to the previous summit that was held in February.  We will be allocating more time to the breakout sessions and less time to static talks.  However, the group will be much larger than the previous summit so this document serves as a way to add discussion points for the breakout sessions.
docs/content/docs/dockercon-summit.md:12: trailing whitespace.
+If you would like to add a discussion point to the agenda, submit a PR adding it to the list below.  A simple one line sentence is enough or expand if needed.
docs/content/docs/dockercon-summit.md:21: trailing whitespace.
+* Since containerd is one of the bottom bricks in the stack, how can we setup automated integration tests for consumers of containerd?
docs/content/docs/dockercon-summit.md:26: trailing whitespace.
+* How to support multi-OS docker images, for example, Linux Vs Windows using one graph driver plugin properly?
docs/layouts/partials/footer.html:11: new blank line at EOF.
docs/source/less/flexbox.less:78: trailing whitespace.
+// <number>
docs/source/less/flexbox.less:89: trailing whitespace.
+// <width>
docs/source/less/flexbox.less:99: trailing whitespace.
+// flex-start | flex-end | center | space-between | space-around
docs/source/less/flexbox.less:109: trailing whitespace.
+// flex-start | flex-end | center | space-between | space-around | stretch
docs/source/less/flexbox.less:119: trailing whitespace.
+// flex-start | flex-end | center | baseline | stretch
docs/source/less/flexbox.less:129: trailing whitespace.
+// auto | flex-start | flex-end | center | baseline | stretch
docs/source/less/layout.less:183: trailing whitespace.
+  .details, .terminal {
docs/source/less/layout.less:184: trailing whitespace.
+    width: 100%;
docs/source/less/layout.less:195: trailing whitespace.
+  .terminal {
docs/source/less/reset.less:1: trailing whitespace.
+/* http://meyerweb.com/eric/tools/css/reset/
docs/source/less/reset.less:15: trailing whitespace.
+article, aside, canvas, details, embed,
docs/source/less/reset.less:16: trailing whitespace.
+figure, figcaption, footer, header, hgroup,
docs/source/less/reset.less:27: trailing whitespace.
+article, aside, details, figcaption, figure,

Also, the docs/Makefile needs the header:

#   Copyright The containerd Authors.

#   Licensed under the Apache License, Version 2.0 (the "License");
#   you may not use this file except in compliance with the License.
#   You may obtain a copy of the License at

#       http://www.apache.org/licenses/LICENSE-2.0

#   Unless required by applicable law or agreed to in writing, software
#   distributed under the License is distributed on an "AS IS" BASIS,
#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
#   See the License for the specific language governing permissions and
#   limitations under the License.

@lucperkins
Copy link
Contributor Author

@ehazlett Done. I also added a .editorconfig file that should cut down on those issues in the future.

@codecov-io
Copy link

codecov-io commented May 15, 2018

Codecov Report

Merging #2342 into master will decrease coverage by 3.74%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2342      +/-   ##
==========================================
- Coverage   44.98%   41.24%   -3.75%     
==========================================
  Files          92       66      -26     
  Lines        9340     7787    -1553     
==========================================
- Hits         4202     3212     -990     
+ Misses       4459     4071     -388     
+ Partials      679      504     -175
Flag Coverage Δ
#linux ?
#windows 41.24% <ø> (ø) ⬆️
Impacted Files Coverage Δ
snapshots/native/native.go 1.8% <0%> (-42.09%) ⬇️
oci/spec.go 0% <0%> (-40%) ⬇️
archive/tar.go 16.85% <0%> (-26.2%) ⬇️
metadata/snapshot.go 21.53% <0%> (-24.28%) ⬇️
oci/spec_opts.go 58.46% <0%> (-6.16%) ⬇️
content/local/writer.go 51.57% <0%> (-1.06%) ⬇️
gc/scheduler/scheduler.go 66.34% <0%> (-0.97%) ⬇️
oci/spec_unix.go
mount/mountinfo_linux.go
sys/fds.go
... and 23 more

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 b511c39...d1503dc. Read the comment docs.

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Luc Perkins <lucperkins@gmail.com>
@crosbymichael
Copy link
Member

LGTM

@caniszczyk
Copy link
Contributor

nice work @lucperkins

@crosbymichael crosbymichael merged commit 257d74f into containerd:master May 16, 2018
@dnephin
Copy link
Contributor

dnephin commented May 16, 2018

It looks like this broke all the links in the README which pointed at github ./docs (including the banner).

Where is this page https://github.com/containerd/containerd/blob/master/docs/content/docs/getting-started.md available on https://containerd.io ? I don't see any links to it, and I can't seem to guess the url from the path.

@estesp
Copy link
Member

estesp commented May 16, 2018

Hmm..looks like we didn't catch the few markdown files which were effectively mixed into the docs/ dir, which was doing double duty between website files and GH markdown docs.

The getting started was not intended for the website as it was just a doc for GH linked from the master README.md. We need a follow-up PR that pulls non-website files back into docs/ I guess.

@dnephin
Copy link
Contributor

dnephin commented May 16, 2018

Just a suggestion, maybe the hugo site root could move to docs/website ?

@lucperkins
Copy link
Contributor Author

@dnephin @estesp My bad. I assumed that those files were meant to be eventually integrated into the website and didn't realize that there are active links to them. PR forthcoming.

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.

9 participants