check for circular aliases#25502
Conversation
✅ Deploy Preview for docsdocker ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The circular alias detection script is well-structured and integrates cleanly with the existing CI pipeline. The logic correctly identifies self-referential aliases across both manuals/ and standard Hugo paths. One minor latent risk noted inline.
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
dvdksn
left a comment
There was a problem hiding this comment.
Hey, I noticed this PR only now. I think there is a better way to detect these. We already process the aliases in the home.redirects.json template. As part of generating the redirects.json file, we could have hugo compare aliases and permalink, and errorf if there's a conflict. Something like:
{{- range hugo.Sites -}}
{{- range .Pages -}}
{{- if .Params.aliases -}}
+ {{- $page := . -}}
{{- $target := .RelPermalink -}}
{{- range .Params.aliases -}}
- {{ $redirects.SetInMap "paths" . $target }}
+ {{- $alias := . -}}
+ {{- if eq (strings.TrimSuffix "/" $alias) (strings.TrimSuffix "/" $target) -}}
+ {{- errorf "alias %q in %s resolves to the page itself" $alias $page.File.Path -}}
+ {{- end -}}
+ {{- $redirects.SetInMap "paths" $alias $target -}}
{{- end -}}
{{- end -}}
{{- end -}}
{{- end -}}|
That took me way longer than it should of to realize that it fails the netlify check and not the build one. |
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The PR adds a build-time self-referential alias check to the Hugo redirect template. The logic is sound for this repo's alias conventions (all aliases use leading slashes, matching 's format), so the equality check after stripping trailing slashes is effective.
No issues found in the changed code.
Signed-off-by: Craig Osterhout <craig.osterhout@docker.com>
9216e08 to
387c7fb
Compare
Description
Added check for circular aliases.
I've done it a few times. Move something and add an alias. Then move it back and forget to remove the alias.
Currently there is a circular alias, so it should fail successfully.
Related issues or tickets
Reviews