update link checker exclude patterns #73
Conversation
Signed-off-by: Joshua Li <[email protected]>
joshuali925
requested review from
lezzago,
anirudha,
ruanyl,
SuZhou-Joe,
xluo-aws,
gaobinlong,
Hailong-am,
wanglam and
raintygao
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #73 +/- ##
===========================================
+ Coverage 58.67% 74.29% +15.61%
===========================================
Files 46 43 -3
Lines 1089 1023 -66
Branches 252 238 -14
===========================================
+ Hits 639 760 +121
+ Misses 441 262 -179
+ Partials 9 1 -8 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Joshua Li <[email protected]>
Signed-off-by: Joshua Li <[email protected]>
Signed-off-by: Joshua Li <[email protected]>
.github/workflows/links_checker.yml
Outdated
| - name: Load Excludes | ||
| run: | | ||
| LYCHEE_EXCLUDE=$(sed -e :a -e 'N;s/\n/ --exclude /;ta' OpenSearch-Dashboards/.lycheeexclude) | ||
| LYCHEE_EXCLUDE=$(curl -fsSL https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/HEAD/.lycheeexclude | cat - .lycheeexclude | grep -v '^#\|^$' | sed -e :a -e 'N;s/\n/ --exclude /;ta') |
There was a problem hiding this comment.
Shall we have a repo-specific .lycheeexclude instead of rely on the external? I guess we can start with a small list of .lycheeexclude first?
There was a problem hiding this comment.
it's already in the PR
dashboards-assistant/.lycheeexclude
Lines 1 to 5 in baa1ab9
There was a problem hiding this comment.
Yes, but do we still need to download from https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/HEAD/.lycheeexclude?
There was a problem hiding this comment.
Yes, but do we still need to download from
https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/HEAD/.lycheeexclude?
we don't want to maintain common excludes in two places, the mainly part should from OSD core and we have patch in this repo.
There was a problem hiding this comment.
Yes, but do we still need to download from
https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/HEAD/.lycheeexclude?we don't want to maintain common excludes in two places, the mainly part should from OSD core and we have patch in this repo.
But the .lycheeexclude in OSD repo is not common excludes as I understand, it's specific to OSD repo, am I right?
There was a problem hiding this comment.
it's specific to OSD repo
agree, it is specific to OSD. But dashboards-assistant is a plugin of OSD, .lycheeexclude defined in OSD should suitable for plugins.
There was a problem hiding this comment.
i think the OSD list makes sense, but we are the only one using it besides OSD. It was added by db292f8, my PR just avoids the whole repo clone for faster CI and debugging. @SuZhou-Joe any specific reason that you added the .lycheeexclude from OSD?
There was a problem hiding this comment.
Actually I copy the lint-checker from OSD, and thus I use the .lycheeexclude from OSD as a reference to keep dashboards-assistant align with OSD. It is OK that we remove this step or merge some repo-specific exclude config with OSD's.
There was a problem hiding this comment.
i see, in that case i'll remove it. the OSD one contains links like github.com, it excludes 404 checks for github links in this repo. without it CI would be able to catch outdated github URLs
Signed-off-by: Joshua Li <[email protected]>
Signed-off-by: Joshua Li <[email protected]>
Description
Issues Resolved
partially #72
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.