Skip to content

fix: gh icon as relative path#170

Merged
joshmoore merged 1 commit into
ome:mainfrom
jo-mueller:relative-gh-icon-path
Jul 3, 2026
Merged

fix: gh icon as relative path#170
joshmoore merged 1 commit into
ome:mainfrom
jo-mueller:relative-gh-icon-path

Conversation

@jo-mueller

Copy link
Copy Markdown
Contributor

Fix for the mildly annoying render of the authorlist over at this issue ome/ngff#553

@lubianat @joshmoore

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Automated Review URLs

Comment thread pre_build.py

if "github" in author:
name += f" [<img src=\"{github_icon}\" alt=\"GitHub\" height=12 width=12 style=\"vertical-align: middle;\"/>](https://github.com/{author['github']})"
name += f" [<img src=icons/invertocat.svg alt=\"GitHub\" height=12 width=12 style=\"vertical-align: middle;\"/>](https://github.com/{author['github']})"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this actually fix it? it seems that it does not find this path on the rendered version

Image

maybe using https://github.githubassets.com/favicons/favicon.svg is safer?

on ome/ngff#552 it looks like this to me:

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over here this results in
image

@jo-mueller jo-mueller Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which is why I went for the slightly altered, but stored in the repo version of the octocat.

will this actually fix it?

I hope so, because the path to the icon was previously written as an absolute path (/path...), which worked in the ngff-spec repo, where root and relative path are the same. Hopefully turning it into a relative path will fix it though! But only one way to find out...

@joshmoore
joshmoore merged commit a447097 into ome:main Jul 3, 2026
4 checks passed
@jo-mueller jo-mueller mentioned this pull request Jul 3, 2026
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.

3 participants