lycheeverse / lychee-action

Github action to check for broken links in Markdown, HTML, and text files using lychee, a fast link checker written in Rust.
https://lychee.cli.rs
Apache License 2.0
292 stars 39 forks source link

Allow to store `.lycheeignore` in workflows folder #205

Closed keunes closed 2 months ago

keunes commented 10 months ago

Hello,

Thanks for making this GH action available!

Currently, the .lycheeignore file has to be stored in root. I may be nitpicking a bit, but it's 'polluting' the root folder a bit. I would like to request that also the workflows folder .github/workflows is checked for this file, so I can place it there. That way it sits nicely with it's parent workflow file.

mre commented 10 months ago

Right now, the path to the .lycheeignore file is hard-coded: https://github.com/lycheeverse/lychee/blob/master/lychee-bin/src/options.rs#L17

This was done deliberately modeled after .gitignore and .dockerignore and to keep things simple. (There is no way to change the ignore-file in git or Docker, either.)

There is a longer discussion at https://github.com/lycheeverse/lychee/issues/648, specifically https://github.com/lycheeverse/lychee/issues/648#issuecomment-1195034686 summarizes our current reasoning on the matter.

I can see three ways going forward:

  1. Stick to the current implementation and enforce .lycheeignore in the project root: simple, but pollutes the root folder
  2. Add a config option to lychee to change the path: would solve the problem in a general way, but would the .lycheeignore in the root folder be ignored if the user specified a different path? There's a similar discussion on .dockerignore here: https://github.com/moby/moby/issues/12886
  3. Fix the issue in this action: check if there is a .github/workflows/.lycheeignore file and use it if no .lycheeignore file exists in the root. (But what if it does?)

I gravitate towards 1, unless we can find a good rationale for 2 or 3. The main issue is, that I want to avoid ambiguity. If a .lycheeignore file exists in multiple places, the user would have to know lychee's behavior in such a case. (Is one file ignored? Are both files loaded? What is the hierarchy?)

@keunes, do other tools support storing their configs in .github/workflows? I'd be curious to take a look.

mre commented 10 months ago

@lebensterben fyi

keunes commented 10 months ago

Thanks for the explanation and for sharing options you see, @mre!

Let me start with saying that I'm not a developer and just fumbling around with AntennaPod. In fact it was someone else who set up the GitHub action for the link-checker.

Looking at the options you present:

  1. (keep as is): naturally I'm not in favour, else I wouldn't have created the issue :D
  2. (option to specify ignore-file location): possible, would give Lychee users nice flexibility
  3. (address in action):
    1. Ignore 1 file if the other is present: not very transparent, users in this situation might miss why the action doesn't ignore certain links and would probably have to look carefully in the logs.
    2. Load both files: I'd gravitate towards this. Possible downside of user 'forgetting' where a specific URL ignore has been defined. But no risk of conflicts between the two files, so that's fine. And it's easy for user (no need to specify additional options).
    3. [new] Throw an error if both are present: not very user friendly but more transparent than 3i
mre commented 8 months ago

I like 3.2 (merge lycheeignores), but the workflows folder is only relevant in a GitHub action context. So we need to have a special case for lychee-action as we can't assume the workflows folder exists locally when the CLI tool is used. At the end, it's the same as option 2.

I wouldn't mind to add a config option, but we'd have to be careful about the option name, because it can be misleading and it bit us in the past: it was called --exclude-file, but people (rightfully) assumed that it mean "exclude this single file". This option is deprecated now and replaced by --exclude-path. My vote goes to --lycheeignore-path or even no option at all and just an environment variable like LYCHEE_IGNORE_PATH. The more explicit, the better.

Any opinions?

mre commented 2 months ago

Thank you very much for your input and the thoughtful discussion regarding the placement of the .lycheeignore file. After reviewing the options and considering the implications of each, I've decided to stick with the current implementation, where the .lycheeignore file is located in the project root. This approach keeps the tool's behavior consistent and avoids potential confusion that could arise from having multiple ignore file locations.

While I understand the concerns about clutter in the root directory, maintaining simplicity and avoiding ambiguity in tool usage ultimately takes precedence. I appreciate your understanding, and please know that your suggestions have been invaluable in evaluating this decision.

Hope that's fine with you, @keunes.