[INFRA] Use linkchecker (from a dedicated docker image) to check all URLs by yarikoptic · Pull Request #293 · bids-standard/bids-specification

yarikoptic · 2019-08-02T19:05:57Z

Supersedes #79

TODOs

make sure it works
possibly change tag and transfer docker image under bids-standard organization on hub.docker.org
fix broken internal (anchored) URLs
add --check-extern option
fix broken external URLs
fixup what broke (thanks travis)

…hecker

…cated docker image as a separate job

…ty!)

yarikoptic · 2019-08-05T14:14:00Z

@sappelhoff WOOHOO -- I finally won (and learned a bit of circle-ci spec ;)). Now I will fixup broken URLs to make it all green!

… readable -- demanded by linkchecker - note: it is important to have trailing / after site/ for linkchecker

apparently my anchor fix to linkchecker is not complete. In multithreaded mode (the beauty of linkchecker) it brings up false positives reporting some anchors which are valid. See e.g. https://circleci.com/gh/bids-standard/bids-specification/911?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

sappelhoff · 2019-08-05T14:40:02Z

super cool, thanks for investing the effort! This looks really good and your to-dos seem reasonable.

+1 to migrate the dockerimage to "bids": https://hub.docker.com/u/bids

…e task example

…pdf describing differences I found no better URL upon a quick search

yarikoptic · 2019-08-05T15:02:01Z

+1 to migrate the dockerimage to "bids": https://hub.docker.com/u/bids

should I just try to push there using the same name:tag or any recommendations ? (not sure if I have super powers for that, but can try)

yarikoptic · 2019-08-05T15:37:56Z

WOOHOO #2 -- all green! so I am done with this, besides may be pushing docker image to under bids/ on docker hub. Unfortunately the solution is not ideal (requires more fixups to linkchecker so threading is disabled) but seems to work.
If you see ways to improve -- you are welcome to push directly into my branch, I need to switch to other things todo.

yarikoptic · 2019-08-08T17:30:47Z

ok, conflicts are piling up -- review/instructions from others to see this merged would be appreciated!

yarikoptic · 2019-08-08T17:31:28Z

attn: @sappelhoff

+1 to migrate the dockerimage to "bids": https://hub.docker.com/u/bids

should I just try to push there using the same name:tag or any recommendations ? (not sure if I have super powers for that, but can try)

* origin/master: [DOC] Auto-generate changelog entry for PR bids-standard#272 Apply suggestions from code review reorder sections as chris suggested STY: Move to passive voice/imperative mood consistent use of <label> Apply suggestions from code review right align cells and formtting add general section on entities fix linter some cleanup/clarifying try links via tiny headings try fix links try links transpose entity table

sappelhoff · 2019-08-09T11:54:19Z

I was leaving this open for a bit to see if other people want to review this @yarikoptic :-)

I have one open question: What is the point of tools/linkchecker-docker/neurodocker-build.sh?

yarikoptic · 2019-08-09T13:52:45Z

I was leaving this open for a bit to see if other people want to review this @yarikoptic :-)

I have one open question: What is the point of tools/linkchecker-docker/neurodocker-build.sh?

that is how that docker image recipe (Dockerfile) is built -- using neurodocker.
Benefits -- could be multiple, e.g. with slight tune up I could also build Singularity recipe, and neurodocker's recipe takes care about cleaning things up thus minimizing the image/layers.
I guess I could/should just save it and commit so people could build their own image without requiring neurodocker being installed?

sappelhoff · 2019-08-09T13:56:35Z

that is how that docker image recipe (Dockerfile) is built -- using neurodocker.

okay, I had suspected something like this --> so is your docker image (linkchecker) calling this script? Because I don't see circleci calling it at any time 🤔

I guess I could/should just save it and commit so people could build their own image without requiring neurodocker being installed?

Could you please clarify what you mean by that?

yarikoptic · 2019-08-09T14:05:44Z

My image is specified for that run in circle ci config https://github.com/bids-standard/bids-specification/pull/293/files#diff-1d37e48f9ceff6d8030570cd36286a61R34

Now it is just pipped into docker build, so never saved. I could save it to a file and commit it first

sappelhoff · 2019-08-09T14:10:22Z

My image is specified for that run in circle ci config

yes, but according to what I understand, this tells CircleCi to download your image from dockerhub

Now it is just pipped into docker build, so never saved. I could save it to a file and commit it first

I think I just don't understand why we need to add the build of the docker image to this repo 😕 ... what would break if we simply removed neurodocker-build.sh from the new tools/linkchecker-docker directory that you created?

yarikoptic · 2019-08-09T17:46:14Z

Now it is just pipped into docker build, so never saved. I could save it to a file and commit it first

I think I just don't understand why we need to add the build of the docker image to this repo ... what would break if we simply removed neurodocker-build.sh from the new tools/linkchecker-docker directory that you created?

it is not building docker image here on CI or travis anywhere. It is just the recipe on HOW that image could be built happen we need to update it (e.g. to a new patched or not version of linkchecker). without it, there would be no clear way on how to do that

sappelhoff · 2019-08-10T07:25:53Z

it is not building docker image here on CI or travis anywhere. It is just the recipe on HOW that image could be built happen we need to update it (e.g. to a new patched or not version of linkchecker). without it, there would be no clear way on how to do that

Ahhh it's what you did prior to pushing the image to your dockerhub ... I got it. Thanks for explaining :-)

Now I'd be ready to merge this PR, but you mentioned something along the lines

I guess I could/should just save it and commit so people could build their own image without requiring neurodocker being installed?

--> My take on this is: People who would do this kind of stuff would have no issues getting neurodocker first ... so from my side, everything is fine. :-)

sappelhoff · 2019-08-12T15:02:40Z

I don't want to delay this PR any further. All future improvements can be done in a separate PR,

this is a nice enhancement as it stands ... thank you very much @yarikoptic!

yarikoptic · 2019-08-12T15:43:09Z

THANKS!

yarikoptic added 2 commits August 2, 2019 15:00

ENH: Neurodocker script to create docker container with patched linkc…

95a46d5

…hecker

ENH: place built site/ into workspace and run linkchecker from a dedi…

649e5ec

…cated docker image as a separate job

yarikoptic force-pushed the bf-links-docker branch 4 times, most recently from bf13283 to 349e386 Compare August 5, 2019 13:47

BF: state that linkchecker requires build (otherwise workspace is emp…

5107ae0

…ty!)

yarikoptic force-pushed the bf-links-docker branch 3 times, most recently from 5f8146d to f46ab03 Compare August 5, 2019 14:10

yarikoptic added 2 commits August 5, 2019 10:18

BF: add linkchecker command (entered image), and chmod entire ~ to be…

64faa85

… readable -- demanded by linkchecker - note: it is important to have trailing / after site/ for linkchecker

BF: fixed up #anchored references in two files

f795519

yarikoptic force-pushed the bf-links-docker branch from f46ab03 to f795519 Compare August 5, 2019 14:21

yarikoptic requested a review from sappelhoff as a code owner August 5, 2019 14:21

yarikoptic added 2 commits August 5, 2019 10:33

BF: URL to mindboggle should have no www.

7d329ff

yarikoptic added 4 commits August 5, 2019 10:41

BF: Cognitive atlas urls went from /term/ to /task/ for the rest stat…

1d73e7b

…e task example

BF: Replaced dead link to fdt_dtifit with URL to FDT/UserGuide

f461c26

BF: Replace dead link to download of ICBM 452T1 to some lame 1 page .…

e25ae72

…pdf describing differences I found no better URL upon a quick search

ENH: add a separate call to check external URLs

ef6801c

yarikoptic requested a review from chrisgorgo as a code owner August 5, 2019 14:51

BF: fixed up introduced problems to tables alignment

54a3469

sappelhoff requested a review from effigies August 5, 2019 16:18

sappelhoff previously approved these changes Aug 5, 2019

View reviewed changes

yarikoptic dismissed sappelhoff’s stale review via 4432d90 August 8, 2019 21:15

sappelhoff approved these changes Aug 9, 2019

View reviewed changes

sappelhoff merged commit fc2638c into bids-standard:master Aug 12, 2019

franklin-feingold added a commit that referenced this pull request Aug 12, 2019

[DOC] Auto-generate changelog entry for PR #293

3f9dd7d

sappelhoff mentioned this pull request Aug 12, 2019

[FIX] 404 error link for fsl dtifit #292

Closed

sappelhoff mentioned this pull request Apr 6, 2020

[WIP] [ENH] Provenance BEP028 #439

Closed

sappelhoff changed the title ~~[INFRA+FIX] Use linkchecker (from a dedicated docker image) to check all URLs~~ [INFRA] Use linkchecker (from a dedicated docker image) to check all URLs Jul 27, 2022

yarikoptic mentioned this pull request Apr 19, 2024

Make extensions identifiers contain . to separate extensions where multiple used #1755

Merged

yarikoptic deleted the bf-links-docker branch April 30, 2024 23:57

Conversation

yarikoptic commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yarikoptic commented Aug 5, 2019

Uh oh!

sappelhoff commented Aug 5, 2019

Uh oh!

yarikoptic commented Aug 5, 2019

Uh oh!

yarikoptic commented Aug 5, 2019

Uh oh!

yarikoptic commented Aug 8, 2019

Uh oh!

yarikoptic commented Aug 8, 2019

Uh oh!

sappelhoff commented Aug 9, 2019

Uh oh!

yarikoptic commented Aug 9, 2019

Uh oh!

sappelhoff commented Aug 9, 2019

Uh oh!

yarikoptic commented Aug 9, 2019

Uh oh!

sappelhoff commented Aug 9, 2019

Uh oh!

yarikoptic commented Aug 9, 2019

Uh oh!

sappelhoff commented Aug 10, 2019

Uh oh!

sappelhoff commented Aug 12, 2019

Uh oh!

yarikoptic commented Aug 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yarikoptic commented Aug 2, 2019 •

edited

Loading