Merge lp:~mterry/update-manager/update-at-start into lp:update-manager
| Status: | Merged |
|---|---|
| Approved by: | Barry Warsaw on 2012-06-25 |
| Approved revision: | 2441 |
| Merged at revision: | 2504 |
| Proposed branch: | lp:~mterry/update-manager/update-at-start |
| Merge into: | lp:update-manager |
| Diff against target: |
896 lines (+148/-397) 8 files modified
UpdateManager/UnitySupport.py (+0/-9) UpdateManager/UpdateManager.py (+22/-168) UpdateManager/UpdateProgress.py (+84/-0) UpdateManager/backend/InstallBackendAptdaemon.py (+11/-5) data/com.ubuntu.update-manager.gschema.xml.in (+0/-5) data/gtkbuilder/UpdateManager.ui (+14/-206) data/update-manager.convert (+0/-1) update-manager (+17/-3) |
| To merge this branch: | bzr merge lp:~mterry/update-manager/update-at-start |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Barry Warsaw (community) | Approve on 2012-06-25 | ||
| Michael Vogt | 2012-06-01 | Pending | |
|
Review via email:
|
|||
Description of the Change
Implements the 'update on start' aspect of the new spec.
This means that all ways to manually start an update are gone from the UI. We no longer warn about how long it's been since the last update, etc.
I've added a new class to handle the initial progress dialog. It just seemed cleaner to me to do it that way rather than introduce more possible states to the main dialog's class. Especially since so little of that class is relevant to the progress dialog.
The progress dialog doesn't look quite like it does in the spec yet. That will require changes to aptdaemon's progress dialog widget. But those can be done separately.
| Michael Terry (mterry) wrote : | # |
- 2439. By Michael Terry on 2012-06-13
-
merge from trunk
| Barry Warsaw (barry) wrote : | # |
When I run ./update-manager from your branch I get the following errors on the console:
% ./update-manager
Traceback (most recent call last):
File "/usr/lib/
raise AttributeError(
AttributeError: Handler on_button_
Traceback (most recent call last):
File "/usr/lib/
raise AttributeError(
AttributeError: Handler on_checkbutton_
Probably some additional left over cruft that isn't removed in your branch?
Also, you mention a spec, but I'm not aware of it. Do you have a link?
Minor misspelling at the top of UpdateProgress.py
You might want to run pyflakes over UpdateProgress.py. Unused imports include Gtk, _, ngettext
Also, you can probably collapse the .Core.utils import onto one line
Spaces around the = sign when setting os.environ[]
4 space indents please! :)
Could you add some tests for the new functionality and files?
| Michael Terry (mterry) wrote : | # |
The spec is https:/
The AttributeError is likely because you need to point at the new glade files too (update-manager isn't smart enough to look in the local location). So either futz with XDG_DATA_DIRS or just do "sudo cp data/gtkbuilder/* /usr/share/
I'll update UpdateProgress.py and fix the nits. You're killing me with the 4-indents. Not that I care, but update-manager source is very inconsistent with 2 or 4, even inside a file, so I've tried to always stick with 2 in all my pending branches. :(
- 2440. By Michael Terry on 2012-06-19
-
review style nits and pyflakes fixes
| Michael Terry (mterry) wrote : | # |
OK, I've updated to fix nits. I'm not going to do the 4-indent change, because that would cause massive headache with all my chain of pending merges. I'd rather just do a big flag-day indent fixup branch at the end, which will fix existing inconsistencies too.
As for tests, I'd like to talk to mvo first about what his plans were for tests going forward. (tests right now are very hodge-pogde; some rely on internal test-only flags for detecting if a dialog was shown, some just use direct imports of modules and poke around, I don't think any use a UI-driving framework) It's fine if that's a blocker, but it's not something I can fix right this second.
| Michael Vogt (mvo) wrote : | # |
On Tue, Jun 19, 2012 at 09:56:18PM -0000, Michael Terry wrote:
> The spec is https:/
>
> The AttributeError is likely because you need to point at the new glade files too (update-manager isn't smart enough to look in the local location). So either futz with XDG_DATA_DIRS or just do "sudo cp data/gtkbuilder/* /usr/share/
Alternatively:
$ ./update-manager --data-dir ./data
should work too.
> I'll update UpdateProgress.py and fix the nits. You're killing me with the 4-indents. Not that I care, but update-manager source is very inconsistent with 2 or 4, even inside a file, so I've tried to always stick with 2 in all my pending branches. :(
Yeah, its a big pain, given the restructure that is going on, it
sounds like a good time to make it consistently 4 spaces, wdyt?
Cheers,
Michael
| Michael Vogt (mvo) wrote : | # |
On Tue, Jun 19, 2012 at 10:07:18PM -0000, Michael Terry wrote:
> OK, I've updated to fix nits. I'm not going to do the 4-indent change, because that would cause massive headache with all my chain of pending merges. I'd rather just do a big flag-day indent fixup branch at the end, which will fix existing inconsistencies too.
That sounds like a reasonable approach.
> As for tests, I'd like to talk to mvo first about what his plans were for tests going forward. (tests right now are very hodge-pogde; some rely on internal test-only flags for detecting if a dialog was shown, some just use direct imports of modules and poke around, I don't think any use a UI-driving framework) It's fine if that's a blocker, but it's not something I can fix right this second.
Indeed, there is no UI-driving framework, my experience with all of
them wasn't that great, so its importing and poking around at the
modules/classes. I'm happy about u-m moving towards a UI-driving
framework, suggestions for a good one welcome.
Cheers,
Michael
| Michael Terry (mterry) wrote : | # |
<mterry> mvo, darn, I was hoping you had been hiding some amazing UI driving framework that you had been too lazy to use. :-/
<mterry> mvo, the best I've used is ldtp, but it's very rickety
<mvo> mterry: yeah, excatly, I wasn't overly impressed with that one :/
<mterry> mvo, well, I'm happy to write various tests, but since doing so might involve various code changes (for instrumentation) and since code is moving around a lot in my branches, would you mind if I did a big test branch after all these intermediate branches land? (ala the indentation one)
<mvo> mterry: that is fine with me (for the given reasons)
So I'll start working on an indentation branch and a testing branch after my branches land on trunk. I believe that means there's no outstanding issue with this particular branch. Could I get a re-review?
| Ken VanDine (ken-vandine) wrote : | # |
I got a very minor merge conflict for UpdateManager/
<<<<<<< TREE
if self._get_
if ago_minutes is not None and ago_minutes > self.NO_
# add timer to ensure we update the information when the
# last package count update was performed
=======
>>>>>>> MERGE-SOURCE
I manually fixed the merge and tested. After applying updates I noticed the "Details of updates" expander doesn't get hidden. Perhaps that is expected at this point and fixed in another branch.
Still reviewing... more to come later.
| Barry Warsaw (barry) wrote : | # |
On Jun 22, 2012, at 06:36 PM, Ken VanDine wrote:
>I got a very minor merge conflict for UpdateManager/
>
><<<<<<< TREE
> if self._get_
> text_label_main = self._get_
> ago_minutes = self._get_
> if ago_minutes is not None and ago_minutes > self.NO_
> text_header = _("Software updates may be available for your computer.")
> # add timer to ensure we update the information when the
> # last package count update was performed
> GObject.
>=======
>>>>>>>> MERGE-SOURCE
I get the same conflict against trunk. I suppose that chunk of code is
supposed to be removed?
| Barry Warsaw (barry) wrote : | # |
On Jun 19, 2012, at 10:07 PM, Michael Terry wrote:
>OK, I've updated to fix nits. I'm not going to do the 4-indent change,
>because that would cause massive headache with all my chain of pending
>merges. I'd rather just do a big flag-day indent fixup branch at the end,
>which will fix existing inconsistencies too.
I think it's fine to defer the reindentation, and I agree the code base is
fairly inconsistent about indentation right now. It would be nice at some
point to do a thorough PEP-8-ification of the code.
>As for tests, I'd like to talk to mvo first about what his plans were for
>tests going forward. (tests right now are very hodge-pogde; some rely on
>internal test-only flags for detecting if a dialog was shown, some just use
>direct imports of modules and poke around, I don't think any use a UI-driving
>framework) It's fine if that's a blocker, but it's not something I can fix
>right this second.
I guess not. This is another thing I'd really like to do at some point,
i.e. refactor the code to be more testable and have better coverage.
Right now your branch is conflicting with trunk. Can you resolve that and
push an update?
| Ken VanDine (ken-vandine) wrote : | # |
On Fri, 2012-06-22 at 18:22 -0400, Barry Warsaw wrote:
> On Jun 22, 2012, at 06:36 PM, Ken VanDine wrote:
>
> >I got a very minor merge conflict for UpdateManager/
> >
> ><<<<<<< TREE
> > if self._get_
> > text_label_main = self._get_
> > ago_minutes = self._get_
> > if ago_minutes is not None and ago_minutes > self.NO_
> > text_header = _("Software updates may be available for your computer.")
> > # add timer to ensure we update the information when the
> > # last package count update was performed
> > GObject.
> >=======
> >>>>>>>> MERGE-SOURCE
>
> I get the same conflict against trunk. I suppose that chunk of code is
> supposed to be removed?
>
Looks like it to me.
--
Ken VanDine
Ubuntu Desktop Integration Engineer
Canonical, Ltd.
- 2441. By Michael Terry on 2012-06-25
-
merge from trunk
| Michael Terry (mterry) wrote : | # |
Yup, the conflicting code just needed to be removed. How about now?
| Barry Warsaw (barry) wrote : | # |
On Jun 25, 2012, at 03:06 PM, Michael Terry wrote:
>Yup, the conflicting code just needed to be removed. How about now?
Looks good, thanks. I've merged this into trunk, but didn't upload it, in
case there are other upcoming changes for u-m. Happy to upload it if you
prefer though.

Note that I also filed https:/ /code.launchpad .net/~mterry/ update- notifier/ use-no- update/ +merge/ 108377 to have update-notifier pass --no-update when launching update-manager.