-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
Warn about apps built against EOL runtimes #1804
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An initial pass; feel free to push back on any changes! But here's what I'm thinking:
- Flatpak seems to use
eol
as the end of life terminology consistently rather thandeprecated
, so I think it makes sense to align our code to that for clarity. - However, End of Life might not really mean what users expect, especially when we're talking about the runtimes and not necessarily the app itself (unless I'm reading this wrong, in which case, oops!).
- So, I made the user-facing copy more generic; this also reflects somewhat how iOS and Android talk about apps made for an older version of the OS, but we can't really say "for an older version of elementary OS" because that's not exactly how runtimes work.
- Lastly, I don't see us needing to care about the number of EOL runtimes, so I refactored that into a bool. Edit: I see you started with an
is_eol
bool so this was basically just reverting back to that while using a more precise name; feel free to change this further if it can be simplified.
I also want to note that this doesn't precisely close #1803; that issue is about a mismatch between the OS version and the runtime version for curated apps, while this PR focuses on EOL runtimes. I think both are valuable, but what @danrabbit is asking for is a little different; if I install an app build against the not-EOL OS 6 runtime on OS 7, it might be useful to still point out that the app was built for OS 6 and thus might be missing out on features/integration. In that situation, I think EOL warning would take precedence, but we'd need to do some version matching and only look at curated apps, etc. I think that is a separate PR, though. |
Yeah for EOL I'd be inclined to use a much larger warning like a floating infobar |
@danrabbit I think I'm still fine with using our content warning space for EOL runtimes, as it keeps the security/updates/expectations information in one place, but I could see that argument, too. 🤔 |
Co-authored-by: Cassidy James Blaede <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, this reverts back further to basically where your is_eol
bool was, but using more precise terminology. Does that make sense?
I must have missed something in the issue 😅 But that shouldn't be a problem to implement. I am preparing a branch based on this PR. |
Co-authored-by: Cassidy James Blaede <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops ignore this accidental double review
Re: #1803
Peek.2022-01-07.12-05.mp4