CI - no absolute links to local pages #849

aduermael · 2016-12-12T19:01:48Z

Describe the proposed changes

New tests to make sure there are no absolute links to docs.docker.com in local pages.

~~makes sure all local pages referenced by links do exist (and not though redirects)~~
~~makes sure all redirects point to existing pages~~

edit: I'll take care of relative links checking in a different PR.

work in progress...

johndmulhausen · 2016-12-12T19:05:18Z

Deploy preview ready!

Built with commit 1a70494

https://deploy-preview-849--docker-docs.netlify.com

aduermael · 2016-12-16T17:25:16Z

Checks have passed! :)
@johndmulhausen can you merge it please?

I would like to start working on new tests for relative links to missing resources and potential redirect loops.

Also, don't you think it's time to enforce tests on master? Almost all PRs to master branch with no Jenkinsfile have been merged. And I would gladly drop comments in the remaining ones to ask for rebase.

Thanks!

aduermael · 2016-12-19T18:10:24Z

@johndmulhausen that PR is good to go! 😊

friism · 2016-12-19T18:25:16Z

lgtm

aduermael · 2016-12-19T18:48:39Z

@friism thanks! :)

johndmulhausen · 2016-12-20T21:25:39Z

@aduermael Right now the CI is failing due to a submodules error:

20:57:32 Checking out Revision cde5c50f47419b74ff69e27a0281403082f6ef7a (PR-849)
20:57:33  > git config core.sparsecheckout # timeout=10
20:57:33  > git checkout -f cde5c50f47419b74ff69e27a0281403082f6ef7a
[Pipeline] sh
20:57:35 [ker_docker.github.io_PR-849-SIO4KRLIUPX4JYS5AYGAJID7R4JI6TFWDRPVZVH7ADS3DWRI6QYA] Running shell script
20:57:36 + git submodule update --init
20:57:36 No submodule mapping found in .gitmodules for path 'tests/src/github.com/gdevillele/frontparser'
[Pipeline] }
[Pipeline] // wrap
[Pipeline] }
[Pipeline] // wrap
[Pipeline] }
[Pipeline] // withDockerRegistry
[Pipeline] }
[Pipeline] // node
[Pipeline] End of Pipeline

GitHub has been notified of this commit’s build result

ERROR: script returned exit code 1
Finished: FAILURE

I think .gitmodules in root needs to exist and have the right entries in this PR.

aduermael · 2016-12-20T21:29:16Z

@johndmulhausen Yes, Go deps are now vendored as submodules (you asked me to do it).
I don't know what's the right way to pull them using Jenkins. But I'll figure this out.

johndmulhausen · 2016-12-21T01:28:01Z

It's great that we're gonna use submodules, thanks for that. Sorry it's creating this problem. Other than that, this PR is looking good. :) @mstanleyjones also did a pass at getting rid of old PRs so we should be able to turn on mandatory Jenkins very soon, though there are a couple bugs that need to be addressed first:

Jenkins jobs taking hours sometimes, and
Jenkins failing on inter-branch PRs, which we're gonna have all the time. That continuous-integration/jenkins/branch check would cause problems.

aduermael · 2016-12-21T19:12:45Z

I fixed the issue with submodules, but meanwhile, new absolute links to docs.docker.com have been introduced... I'm fixing that.

aduermael · 2016-12-21T19:30:07Z

Yay! All checks have passed! 🙂
@johndmulhausen, can you review it please?

Signed-off-by: Adrien Duermael <adrien@duermael.com>

This reverts commit ca54f76. We want to fix the root cause, not the symptoms. So let’s make sure original content is fine instead of fixing it at build. Signed-off-by: Adrien Duermael <adrien@duermael.com>

Signed-off-by: Adrien Duermael <adrien@duermael.com>

aduermael · 2016-12-27T18:58:49Z

@johndmulhausen ?

mdlinville · 2016-12-27T21:58:44Z

docker-for-windows/index.md

@@ -82,7 +82,7 @@ enabled](troubleshoot.md#virtualization-must-be-enabled) in Troubleshooting.
 <p />
 * Nested virtualization scenarios, such as running Docker for Windows on a VMWare or Parallels instance, might work, but come with no guarantees (i.e., not officially supported).
 <p />
-* **What the Docker for Windows install includes**: The installation provides [Docker Engine](https://docs.docker.com/engine/userguide/intro/), Docker CLI client, [Docker Compose](https://docs.docker.com/compose/overview/), and [Docker Machine](https://docs.docker.com/machine/overview/).
+* **What the Docker for Windows install includes**: The installation provides [Docker Engine](/engine/userguide/intro/), Docker CLI client, [Docker Compose](/compose/overview/), and [Docker Machine](/machine/overview/).


Not introduced here, but shouldn't this be a link to a .md file?

@mstanleyjones I have no idea... 😕

mdlinville · 2016-12-27T21:59:12Z

docker-for-windows/index.md

@@ -478,15 +478,15 @@ You can configure options on the Docker daemon in the given JSON configuration f

 ![Docker Daemon](images/docker-daemon.png)

-For a full list of options on the Docker daemon, see <a href="https://docs.docker.com/engine/reference/commandline/dockerd/" target="_blank">daemon</a> in the Docker Engine command line reference.
+For a full list of options on the Docker daemon, see [daemon](/engine/reference/commandline/dockerd/) in the Docker Engine command line reference.


You changed the functionality of this link by removing the target attribute...

@mstanleyjones I can use this syntax with Kramdown: [link](url){:target="_blank"}
I'll fix it now.

mdlinville · 2016-12-27T21:59:44Z

docker-for-windows/index.md


 In that topic, see also:

-* [Daemon configuration file](https://docs.docker.com/engine/reference/commandline/dockerd/#/daemon-configuration-file)
+* [Daemon configuration file](/engine/reference/commandline/dockerd/#/daemon-configuration-file)


Not introduced here, but these should point to the .md file if possible.

@mstanleyjones I can fix it if you're sure we should always link to md files

@mstanleyjones well if I do this the redirect loses the anchor link. You can try by yourself:

https://docs.docker.com/engine/reference/commandline/dockerd/#/daemon-configuration-file

versus

https://docs.docker.com/engine/reference/commandline/dockerd.md#/daemon-configuration-file

The correct Markdown should be:

[Daemon configuration file](/engine/reference/commandline/dockerd.md#daemon-configuration-file)

@mstanleyjones https://docs.docker.com/engine/reference/commandline/dockerd.md#daemon-configuration-file doesn't work either...

maybe it expects relative URLs; ../engine/reference/commandline/docker.md#daemon-configuration-file ?

mdlinville · 2016-12-27T22:00:18Z

machine/concepts.md

@@ -37,7 +37,7 @@ For a complete list of `docker-machine` subcommands, see the [Docker Machine sub

 Users using their own Docker Registry will experience `x509: certificate signed by unknown authority` 
 error messages if their registry is signed by custom root Certificate Authority and it is 
-not registered with Docker Engine. As discussed in the [Docker Engine documentation](https://docs.docker.com/engine/security/certificates/#/understanding-the-configuration)
+not registered with Docker Engine. As discussed in the [Docker Engine documentation](/engine/security/certificates/#/understanding-the-configuration)


As above, link to the .md if possible.

@mstanleyjones Same as above... Losing the anchor link

@mstanleyjones Also, one important point. Currently it is somehow better to use a path of this form: https://docs.docker.com/engine/reference/commandline/dockerd/
Instead of https://docs.docker.com/engine/reference/commandline/dockerd.md

Because the non-md form leads you right to the page you want. While the md form returns a terrible 404 error with a client side JS script that takes you to the non-md path...

We already mentioned that in the past, I really think it's an issue for SEO.

But I also think it makes more sense to link to local markdowns within markdowns. Maybe we should just update these links at build time.

Signed-off-by: Adrien Duermael <adrien@duermael.com>

as requested by @mstanleyjones Signed-off-by: Adrien Duermael <adrien@duermael.com>

aduermael · 2016-12-27T22:40:23Z

@mstanleyjones I added the missing target=_blank. And fixed some links to md files.
But I can't fix them all, as the one containing anchor links would be broken.

Please merge that PR as soon as possible, I don't want absolute links to be re-introduced.

aduermael · 2016-12-27T22:41:28Z

@mstanleyjones @johndmulhausen Also, the Jenkins tests should be made required now. Please please please...

aduermael · 2016-12-28T06:20:03Z

@mstanleyjones about that issue with links that should point to actual markdowns. I'm totally on board with this idea.
But I'm sure it happens in many files, and I would prefer to fix that in a dedicated PR, along with new tests for that particular issue. Because as of today, there's nothing making sure that will never happen again.
So can you please merge that one @mstanleyjones? I'll submit a new one to address that issue.
Thanks!
cc @thaJeztah @johndmulhausen

aduermael · 2016-12-29T19:45:15Z

thank you @mstanleyjones 🙂

UCP storage requirements update

aduermael force-pushed the check-links branch 2 times, most recently from 045c025 to 3e9cd5a Compare December 12, 2016 20:14

aduermael mentioned this pull request Dec 12, 2016

replaced href by src in img tag (v1.7) #852

Merged

aduermael force-pushed the check-links branch 2 times, most recently from 1a70494 to c5fcc1d Compare December 14, 2016 18:22

This was referenced Dec 14, 2016

fixed absolute links #883

Merged

Fixed absolute links in external content #886

Merged

aduermael force-pushed the check-links branch 2 times, most recently from 7f06e48 to ffbdaaf Compare December 16, 2016 02:07

aduermael changed the title ~~CI - check all links to local pages [DO NOT MERGE]~~ CI - check all links to local pages Dec 19, 2016

aduermael changed the title ~~CI - check all links to local pages~~ CI - check absolute links to local pages Dec 19, 2016

aduermael changed the title ~~CI - check absolute links to local pages~~ CI - no absolute links to local pages Dec 19, 2016

aduermael force-pushed the check-links branch from cde5c50 to db2565e Compare December 20, 2016 21:52

aduermael force-pushed the check-links branch from 66ada90 to dae6b14 Compare December 21, 2016 18:52

aduermael added 6 commits December 27, 2016 10:44

added test for absolute links to docs.docker.com

464ce98

Signed-off-by: Adrien Duermael <adrien@duermael.com>

made absolute links to docs.docker.com relative

5b3cbf7

Signed-off-by: Adrien Duermael <adrien@duermael.com>

fixed absolute link in engin/installation/binaries

f9dad1a

Signed-off-by: Adrien Duermael <adrien@duermael.com>

changes in Jenkinsfile

80381c9

Signed-off-by: Adrien Duermael <adrien@duermael.com>

look for href and src attributes in a and img tags

15e07d6

Signed-off-by: Adrien Duermael <adrien@duermael.com>

look for absolute links to docs.docker.com

39ea6e4

Signed-off-by: Adrien Duermael <adrien@duermael.com>

aduermael added 10 commits December 27, 2016 10:44

forgotten absolute links in Dockerfile

f708add

Signed-off-by: Adrien Duermael <adrien@duermael.com>

updated docs.docker.com absolute links test

da5c4d2

Signed-off-by: Adrien Duermael <adrien@duermael.com>

Revert "forgotten absolute links in Dockerfile"

09d4a6f

This reverts commit ca54f76. We want to fix the root cause, not the symptoms. So let’s make sure original content is fine instead of fixing it at build. Signed-off-by: Adrien Duermael <adrien@duermael.com>

exclude archives when testing urls for now

4cbff2c

Signed-off-by: Adrien Duermael <adrien@duermael.com>

CI: don’t build jekyll redirects for now

bb6a13b

Signed-off-by: Adrien Duermael <adrien@duermael.com>

Updated format for comments in Jenkinsfile

ed9b3f4

Signed-off-by: Adrien Duermael <adrien@duermael.com>

tests: Go deps are now vendored as submodules

7cf627c

Signed-off-by: Adrien Duermael <adrien@duermael.com>

Jenkinsfile: init submodules

dc8d5ed

Signed-off-by: Adrien Duermael <adrien@duermael.com>

fixed submodules

6c9d099

Signed-off-by: Adrien Duermael <adrien@duermael.com>

made absolute links to docs.docker.com relative

5e3fbb8

Signed-off-by: Adrien Duermael <adrien@duermael.com>

aduermael force-pushed the check-links branch from 6ffc555 to 5e3fbb8 Compare December 27, 2016 18:44

mdlinville suggested changes Dec 27, 2016

View reviewed changes

aduermael added 2 commits December 27, 2016 14:33

missing “target=_blank”

8f1bc38

Signed-off-by: Adrien Duermael <adrien@duermael.com>

links to md files

51df23e

as requested by @mstanleyjones Signed-off-by: Adrien Duermael <adrien@duermael.com>

mdlinville approved these changes Dec 29, 2016

View reviewed changes

mdlinville merged commit ae629e5 into docker:master Dec 29, 2016

bermudezmt pushed a commit that referenced this pull request Nov 7, 2018

Merge pull request #849 from docker/ucp-storage-req-767

1e5e78f

UCP storage requirements update

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI - no absolute links to local pages #849

CI - no absolute links to local pages #849

aduermael commented Dec 12, 2016 •

edited

Loading

johndmulhausen commented Dec 12, 2016 •

edited

Loading

aduermael commented Dec 16, 2016

aduermael commented Dec 19, 2016

friism commented Dec 19, 2016

aduermael commented Dec 19, 2016

johndmulhausen commented Dec 20, 2016 •

edited

Loading

aduermael commented Dec 20, 2016

johndmulhausen commented Dec 21, 2016 •

edited

Loading

aduermael commented Dec 21, 2016

aduermael commented Dec 21, 2016

aduermael commented Dec 27, 2016

mdlinville Dec 27, 2016

aduermael Dec 27, 2016

mdlinville Dec 27, 2016

aduermael Dec 27, 2016 •

edited

Loading

mdlinville Dec 27, 2016

aduermael Dec 27, 2016

aduermael Dec 27, 2016

mdlinville Dec 27, 2016

aduermael Dec 27, 2016

thaJeztah Dec 27, 2016

mdlinville Dec 27, 2016

aduermael Dec 27, 2016

aduermael Dec 27, 2016

aduermael commented Dec 27, 2016

aduermael commented Dec 27, 2016

aduermael commented Dec 28, 2016

aduermael commented Dec 29, 2016

CI - no absolute links to local pages #849

CI - no absolute links to local pages #849

Conversation

aduermael commented Dec 12, 2016 • edited Loading

Describe the proposed changes

johndmulhausen commented Dec 12, 2016 • edited Loading

aduermael commented Dec 16, 2016

aduermael commented Dec 19, 2016

friism commented Dec 19, 2016

aduermael commented Dec 19, 2016

johndmulhausen commented Dec 20, 2016 • edited Loading

aduermael commented Dec 20, 2016

johndmulhausen commented Dec 21, 2016 • edited Loading

aduermael commented Dec 21, 2016

aduermael commented Dec 21, 2016

aduermael commented Dec 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduermael Dec 27, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduermael commented Dec 27, 2016

aduermael commented Dec 27, 2016

aduermael commented Dec 28, 2016

aduermael commented Dec 29, 2016

aduermael commented Dec 12, 2016 •

edited

Loading

johndmulhausen commented Dec 12, 2016 •

edited

Loading

johndmulhausen commented Dec 20, 2016 •

edited

Loading

johndmulhausen commented Dec 21, 2016 •

edited

Loading

aduermael Dec 27, 2016 •

edited

Loading