open-ephys / bonsai-onix1-docs

Documentation for the OpenEphys.Onix1 Bonsai package
https://open-ephys.github.io/bonsai-onix1-docs/
Creative Commons Attribution Share Alike 4.0 International
0 stars 2 forks source link

Exclude certain URLs for local linkchecks #109

Closed cjsha closed 3 weeks ago

cjsha commented 4 weeks ago

Resolve #74

cjsha commented 4 weeks ago

I was looking at the gh actions - afaik this doesn't affect them because the docfx-utils.ps1 script is never being run with the -a or -l flag. In any case, I don't wouldn't wanna mess those up - I'll let @bparks13 modify those if necessary.

bparks13 commented 4 weeks ago

I already had forgotten that we decoupled the lychee action remotely; it probably isn't worth adding a flag to the utility script, because it won't affect the actions since we are using a totally different system.

I would remove the $remote flag, and just exclude the link everytime it's run locally. The other changes are valid and we should leave the renaming as it is.

I guess we don't even need to update the README, since the only thing that's being changed is the internal call and not the alias, so that's easy.

cjsha commented 4 weeks ago

Why did we decouple it again? Is it for the --verbose? Is it worth recoupling now that we have an option of adding a -r flag for remote builds?

bparks13 commented 4 weeks ago

It's decoupled because there is a Github action already packaged for lychee. If we didn't use the action, we would need to download and install the lychee executable in the Github environment for every run, which seemed like more trouble than it was worth to utilize the utility script. We can definitely revisit this issue though and see if it is worth the effort

cjsha commented 4 weeks ago

Nah. I think that's the way to go for now. I think a "wish list" issue will be doing something similar locally so we don't have to specify the lychee location when calling docfx-utils.ps1. And maybe even also coupling local and remote lychee linkchecks.

In the meanwhile, I think the best solution is as you suggested which is to remove the $remote flag so I will do that.

bparks13 commented 3 weeks ago

Approved, but leaving this open in case we want to revisit and update anything before merging

cjsha commented 3 weeks ago

I don't think I'll have any updates. I already made the "wish list" issue (#114) described in my last comment. If you have any reservations or changes you'd like to make, feel to address them. Otherwise, go ahead and merge.