Restore button updates in partition page.
ClosedPublic
Actions

Authored by kc2bez on Apr 9 2019, 1:16 AM.

Details

Reviewers

tsimonq2
wxl

Maniphest Tasks

T40: Cala segfault when performing action with no selection

Commits

rCALAPACKAGING3758f97ccd03: Restore button updates in partition page.

Summary

Restore the button update status matches the current state. This resolves T40 lp:#1823569

Test Plan

Verify in manual partitioning that the buttons are updated properly.

Diff Detail

Repository

rCALAPACKAGING calamares Packaging

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kc2bez requested review of this revision.Apr 9 2019, 1:16 AM

kc2bez created this revision.

Harbormaster completed remote builds in B3: Diff 4.Apr 9 2019, 1:16 AM

Harbormaster completed remote builds in B3: Diff 4.

Nitpicky, but I would avoid use of Origin as it is optional when Author is present (otherwise, it's required). Additionally, It would include either the URL or the commit, with optional prefixed keywords. That said, if you were going to use it, you would actually use upstream, commit:2b149fb, which would make your Applied-Upstream a bit redundant. I've gotten into the habit of using Applied-Upstream as you can see in the packaging example. At the very least, I would pick one or the other.

This revision now requires changes to proceed.Apr 9 2019, 6:08 AM

Ammend quilt header.

Harbormaster completed remote builds in B5: Diff 6.Apr 9 2019, 6:47 AM

LGTM. BTW, the commit:identifier format would have been fine, though DEP-3 does sort of suggest a URL.

This revision is now accepted and ready to land.Apr 9 2019, 6:51 AM

@wxl could you please go ahead and upload this?

Actually, one thing from me before I'm ACKing this.

Also, while you're at it, please change the commit ref to commit:2b149fb944da8d975c05d562ab55adaa9dec25bb.

Thanks!

debian/changelog
3	Nitpick here, could you change this to `Restore button updates in partition page (LP: #1823569).` ? While that current format probably matches the regex, it's not really consistent. 😄

This revision now requires changes to proceed.Apr 9 2019, 12:40 PM

Uh @tsimonq2 after my comment he actually used a URL instead of commit, so that one comment is irrelevant.

Also the LP bug reference does match the required regex which is why I didn't complain about it. Our own packaging guide (that you wrote) says that's all that needs to be done. If you're going to nitpick THAT much (really??) then you should change the docs.

Ammend the changelog entry to properly reference LP.

Harbormaster completed remote builds in B6: Diff 7.Apr 9 2019, 9:15 PM

wxl accepted this revision.Apr 9 2019, 9:18 PM

If you're going to nitpick THAT much (really??) then you should change the docs.

Sure, I'll change them. Could you upload to Disco, @wxl?

In D3#84, @tsimonq2 wrote:

Could you upload to Disco, @wxl?

When I get home. I got to set up a dev container I can access anywhere, but I don't have my Phab key readily available, so I can't really do that now. If you need it more urgently (might not be a bad idea so we can test and make sure it works), you might want to just do it.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 11 2019, 12:35 AM

Closed by commit rCALAPACKAGING3758f97ccd03: Restore button updates in partition page. (authored by kc2bez, committed by tsimonq2). · Explain Why

This revision was automatically updated to reflect the committed changes.

tsimonq2 changed the visibility from "All Users" to "Public (No Login Required)".Jun 18 2019, 9:22 PM