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

Add out-of-date update recommendation tip #3161

Open
wants to merge 1 commit into
base: 1.15.0-dev
Choose a base branch
from

Conversation

chromatic
Copy link
Member

This requires us to update the expected days to the next release and the date of the current release when we fixate a release. That's why the release process is updated too.

@chromatic chromatic marked this pull request as ready for review November 24, 2022 07:22
@chromatic
Copy link
Member Author

I'm not sure if this is a good idea or not or if the implementation looks sensible. Comments welcome.

@patricklodder patricklodder added this to 🚀 needs review in Review & merge board Nov 24, 2022
@patricklodder
Copy link
Member

I think we need to split this up. I agree with introducing a release cycle. As well, although it must always remain optional to upgrade (because code defines consensus), recording it in the binary as part of the release process, measuring against it and displaying a warning when the binary is likely to be outdated could be acceptable, depending on the text.

I am however not sure that this should be a UI tip, because these tips are only displayed on Qt binaries with wallet enabled, whereas the process would be valid for all installations - including wallet disabled and headless instances. I think that the most important bugfixes and protocol updates are happening outside of wallet functionality. We have a facility that displays a warning for pre-release binaries, and I was thinking that perhaps that would be a better vehicle to extend?

@chromatic
Copy link
Member Author

I like that idea. Extending GetWarnings() makes sense too; my reading of the code suggests it occurs in more places than just startup, which is why I liked the tips idea.

I'll split this into either two PRs or two commits, based on the preference of any developer who cares to comment (I have no preference). First part: just the date code in headers and release process update. Second part: user notification, including text.

@patricklodder
Copy link
Member

First part: just the date code in headers and release process update.
Second part: user notification, including text.

This sounds like a good plan to me.

@chromatic chromatic force-pushed the add-update-tip-with-manual-release-dates branch from ee52fe1 to c923db5 Compare January 7, 2023 20:38
@chromatic
Copy link
Member Author

I've rebased this on top of #3191 and kept the commits as structured so it's easier to review the changes. This seems like an improvement, but I have mixed feelings about the new header. Moving this to a draft so we're not tempted to merge without squash/rebase.

@chromatic chromatic marked this pull request as draft January 7, 2023 20:40
@patricklodder patricklodder removed this from 🚀 needs review in Review & merge board Jan 9, 2023
@chromatic chromatic force-pushed the add-update-tip-with-manual-release-dates branch from 4129acf to f03e0fa Compare January 10, 2023 23:36
@chromatic
Copy link
Member Author

I'm re-running the i686-win CI step because it failed at downloading a ccache deb.

@patricklodder
Copy link
Member

Yeah - sorry I keep on restarting things if I see it fail because it's been terrible (on both bionic and focal hosts) the past month. Not sure what's up.

@chromatic chromatic marked this pull request as ready for review January 22, 2023 02:22
@chromatic
Copy link
Member Author

I think this is feature complete now. Commits need squashing; is there documentation to add?

I'm also interested in feedback on my modifications to the RPC tests. I'm not convinced the implementation is wonderful, but at least it's self-contained.

@patricklodder patricklodder added this to 🚀 needs review in Review & merge board Feb 8, 2023
@chromatic chromatic force-pushed the add-update-tip-with-manual-release-dates branch from 0449f2c to 126380a Compare June 29, 2023 23:42
@chromatic chromatic force-pushed the add-update-tip-with-manual-release-dates branch from 126380a to b419b1f Compare August 31, 2023 03:51
This requires us to update the expected days to the next release and the
date of the current release when we fixate a release. That's why the
release process is updated too.
@chromatic chromatic force-pushed the add-update-tip-with-manual-release-dates branch from b419b1f to b48b581 Compare August 31, 2023 03:51
@chromatic
Copy link
Member Author

I've squashed and rebased.

@patricklodder patricklodder mentioned this pull request Feb 8, 2024
43 tasks
@patricklodder patricklodder removed this from the 1.14.7 milestone Feb 16, 2024
@patricklodder patricklodder changed the base branch from 1.14.7-dev to 1.15.0-dev February 28, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Review & merge board
🚀 needs review
Development

Successfully merging this pull request may close these issues.

None yet

2 participants