-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
[Backport release-24.11] lockbook-desktop: 0.9.15 -> 0.9.16 #373822
[Backport release-24.11] lockbook-desktop: 0.9.15 -> 0.9.16 #373822
Conversation
(cherry picked from commit 6b5bf05)
I doubt that you built or tested anything against the release-24.11 branch when approving so fast. Maybe we should add the failed cherry-pick from #373058 here. |
(cherry picked from commit 01aa1e3)
Open to advice on what I should be testing generally. Here is what's on my mind:
We also release fairly often. But open to advice and updating my models generally. Also wouldn't mind feedback on what the most nix-friendly way to delivery small updates often to our community is. I waited about 6 hours for @r-ryantm pickup our changes, but figured a pr like this may be faster. But I don't want to increase load on nix committers, and I'm not sure it actually is faster. So open to advice on that front as well. |
Sure :)
Yes, this is only about nix specific stuff. You never know - dependencies between master and release-24.11 might be different, so at least the build should be tested. I assume once it builds and you propose this as an upstream dev, I don't need to spend time running it or anything - you'll have taken care of that. But we need to test it builds, so we're not breaking anyone updating.
Well, that's the point: nixpkgs CI does not build. Currently, at least.
Yes, but lockbook itself could also be broken for a specific dependency on a certain version. In that case, this would need investigation.
Yeah, not sure how fast r-ryantm is going to do that either. If you're opening a PR yourself, I think it's easier for both you and the reviewer to put lockbook and lockbook-desktop into one PR, two separate commits. If you then run a nixpkgs-review on the PR (wait for CI's Eval to be through first, so you don't have to eval locally) and post the result, a potential reviewer might be ready to merge already. I would. You could also add some labels like "approved by package maintainer" and "upstream developer", that would make the PR more discoverable and give reviewers a hint directly. Also good would be to post a link to the release notes / changelog in the body of the PR, that helps reviewers check the changes for breaking changes when backporting etc. I'm sure as an upstream developer + package maintainer you have all of those changes on your radar anyway, but it helps making the reviewer aware of that.
Feel free to request review from me for your simple update PRs, ideally with the info above :) (I like request review more than a ping by name in that case) |
Addition: Ideally the request for review comes in when CI has run through already and the review result is there. With everything on the table, I could merge on the spot and not have to keep track of the PR myself. |
|
This was a very helpful message, thank you! Thank you also for bringing over the failing cherry-pick. I'll keep all of the above in mind |
Bot-based backport to
release-24.11
, triggered by a label in #373059.