Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve behavior around --show-redirects #64

Merged
merged 3 commits into from
Jan 23, 2021

Conversation

nfagerlund
Copy link
Contributor

PR #54 added a --show-redirects flag! But the implementation didn't seem quite complete:

  • It adds a redirects count to the summary, but it won't show the details about WHICH links were redirected
    unless there was also at least one error or warning. So if you want to
    actually update your redirected links, you need to temporarily break an
    unrelated link first. 😆
  • If you didn't want info on redirects, you get spammed with it anyway whenever an unrelated error or warning triggers a full-detail report.
  • Redirects aren't detectable in the exit code.

So, this PR has a commit to deal with each of those.

PR filiph#54 added a `--show-redirects` flag! But although it adds a redirects count
to the summary, it won't show the details about WHICH links were redirected
unless there was *also* at least one error or warning. So if you want to
actually update your redirected links, you need to temporarily break an
unrelated link first. 😆

This commit adds "they requested redirects, and there was at least one redirect"
to the list of conditions that can trigger a full report.
If the user asked for alerts on redirects, they presumably want a nonzero exit
code so their CI etc. will recognize redirects as a problem.

Redirects seem more like warnings than errors, since they work fine but are
undesirable.
@filiph
Copy link
Owner

filiph commented Jan 23, 2021

Fantastic, thank you!

@filiph filiph merged commit e79e9bd into filiph:master Jan 23, 2021
@nfagerlund nfagerlund deleted the jan21_really_show_redirects branch January 25, 2021 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants