AppCenter

Merge lp://staging/~jeremywootten/appcenter/restructuring-part3 into lp://staging/~elementary-apps/appcenter/appcenter

  1. restructuring-part3
  2. Merge into appcenter
Proposed by Jeremy Wootten
Status: Merged
Approved by: Danielle Foré
Approved revision: 287
Merged at revision: 295
Proposed branch: lp://staging/~jeremywootten/appcenter/restructuring-part3
Merge into: lp://staging/~elementary-apps/appcenter/appcenter
Prerequisite: lp://staging/~jeremywootten/appcenter/restructuring-part2
Diff against target: 1332 lines (+674/-359)
12 files modified
src/AbstractAppContainer.vala (+9/-31)
src/AbstractAppList.vala (+118/-0)
src/CMakeLists.txt (+4/-0)
src/Core/ChangeInformation.vala (+3/-3)
src/Core/Package.vala (+11/-5)
src/Views/AppInfoView.vala (+29/-32)
src/Views/AppListView.vala (+235/-270)
src/Views/InstalledView.vala (+5/-5)
src/Widgets/AppActionButton.vala (+40/-0)
src/Widgets/AppListRow.vala (+31/-0)
src/Widgets/PackageRow.vala (+19/-13)
src/Widgets/UpdateHeaderRow.vala (+170/-0)
To merge this branch: bzr merge lp://staging/~jeremywootten/appcenter/restructuring-part3
Reviewer Review Type Date Requested Status
Danielle Foré ux Approve
Adam Bieńkowski (community) code Approve
Review via email: mp+303616@code.staging.launchpad.net

Commit message

Refactor AppListView

Description of the change

Further code cleanup - AppListView

To post a comment you must log in.
lp://staging/~jeremywootten/appcenter/restructuring-part3 updated
282. By Jeremy Wootten

Merge changes from part2, resolve conflicts; remove whitespace

283. By Jeremy Wootten

Fix regression in button style; remove redundant code

284. By Jeremy Wootten

Merge changes from trunk r288

Revision history for this message
Danielle Foré (danrabbit) wrote :

Small inline comment about margin

Revision history for this message
Danielle Foré (danrabbit) wrote :

Can you resolve conflicts on this branch and merge trunk please?

review: Needs Fixing
lp://staging/~jeremywootten/appcenter/restructuring-part3 updated
285. By Jeremy Wootten

Fix margins; remove redundant code

Revision history for this message
Danielle Foré (danrabbit) wrote :

As far as I can tell this is all working as expected.

Sorting/headers for upgrades seems to be working correctly (a big improvement over trunk).

Installation progress is persistent across views.

Installing/removing/upgrades all seem to work as expected. (even cancelling specific packages in update all)

review: Approve (ux)
Revision history for this message
Adam Bieńkowski (donadigo) wrote :

Thank you, this is huge improvement over the trunk. I have couple issues with the code style though. I commented on the diff what I would like to be fixed. Mainly I don't like the "AAAAAAA" hardcoded string, maybe decide which row should be first based on the object type? The empty public constructors are not needed here since in Vala they are public by default. The UI / UX seems to not have any issues, I would say that it is an improvement mostly due to not creating widgets everytime the header is displayed.

Code: needs fixing
UI / UX: approve

review: Needs Fixing (code / testing)
lp://staging/~jeremywootten/appcenter/restructuring-part3 updated
286. By Jeremy Wootten

Merge trunk to r293

287. By Jeremy Wootten

Rework sort function; do not use name property for headers

Revision history for this message
Adam Bieńkowski (donadigo) wrote :

There are minor issues still within the code, but I am able to approve it how it is. I would still change the get_name_label definition to include nullable type, but it's not something really important if it works correctly. I didn't test the branch in the latest revision, it's only code review.

review: Approve (code)
Revision history for this message
Danielle Foré (danrabbit) wrote :

I can still confirm that this works as expected

review: Approve (ux)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches