-
Notifications
You must be signed in to change notification settings - Fork 371
Resolve paths for default icon #1210
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
Resolve paths for default icon #1210
Conversation
|
It seems a little weird to add custom code for expanding the Adding a |
|
I was on the edge whether I should expand
|
|
I took a closer look at |
904ce6f to
4a1ee5d
Compare
|
I've implemented a version of the resolution using
|
4a1ee5d to
dc64c31
Compare
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1210 +/- ##
=======================================
Coverage 66.03% 66.03%
=======================================
Files 46 46
Lines 7595 7595
=======================================
Hits 5015 5015
Misses 2580 2580
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I think it's better to keep the |
Currently is always treated as a plain string so no expansion of `~` happens. The real resolution (name of icon vs. path to icon) is done later when the notification is about to be rendered.
dc64c31 to
ca09495
Compare
|
Thanks, this makes it easy to review :) |
This (partially) fixes #1173 - support for expanding variables is done in #1215
default_iconwas erroneously declared as being a plain string while it can also be a full path. Thus changing its type should solve the described issue already (without any side effects I could currently see).