fix front page links and change check_links #318
Conversation
noursaidi
commented
Apr 29, 2022
•
noursaidi
commented
- change check_links to work on hosted (GitHub pages or GitHub) links
- fix check_links so it works when there are two links on the same line
- change front page links to rendered version
bin/check_links
Outdated
| # Rewrite hosted links (e.g. github pages) to local | ||
| rewritten_link = blank_regex_substitutions(link, HOSTED_GITHUB, HOSTED_GITHUB_PAGES) | ||
|
|
||
| if not re.match('https?://', rewritten_link): |
There was a problem hiding this comment.
Optional: My personal preference is re.search and ^ in front of the regex because it's more clear, and also consistent with other re functions.
There was a problem hiding this comment.
Changed all re.match to re.search and added ^ to start of regex
bin/check_links
Outdated
| @@ -8,6 +8,8 @@ import glob | |||
| import re | |||
| import os | |||
|
|
|||
| HOSTED_GITHUB_PAGES = r"https?:\/\/faucetsdn.github.io\/udmi\/" | |||
There was a problem hiding this comment.
For consistency I think it should be r'' ? (single quote)
I think both patterns should have a ^ in front and . for periods in the hostname?
| @@ -60,12 +80,24 @@ def check_links(file_path): | |||
|
|
|||
| if not os.path.exists(resolved_path): | |||
| failing_links.append((file_path, 'link', link)) | |||
|
|
|||
| else: | |||
There was a problem hiding this comment.
Can you please add to the function docstring what we are accomplishing with this change. i.e. when do we need this to occur?
There was a problem hiding this comment.
Extended function docstring to describe new behaviour and added to incline comments
|
@johnrandolph yes, there's a "changes from all commits" button on the files changed page which shows all changes, it seems to default to original commit for some reason. Let me know if I've missed something |
