Concerns about the use of +localpackagediffs in Ubuntu

[Stefano Rivera]

Sorry, this isn't a single bug rather a set of concerns that have me (and others) thinking that Ubuntu Developers should *not* be using +localpackagediffs in its current form. We have a command line tool (syncpackage) using the copyPackage API call, and that should be all most developers need. I know launchpad doesn't like special-casing Ubuntu, can this not be made an archive-admin-only interface?

TLDR: The +localpackagediffs interface looks too powerful and easy to make mistakes with. Can it be made archive-admin-only?

Batch-centric: This interface looks like it was built for Archive Administrators, not individual developers. I can't say I often need to sync more than one package at a time, and when I do, I'm happy to discuss this with Archive Administrators, that's what they're there for.

Ease of making mistakes: The interface has no confirmation when making a single sync, or (I'd imagine, from the massive accidental sync) a mass sync. There's a lot of checkboxes on the screen, and one of them could have been accidentally checked.
Our current sync request tool asks for additional confirmation when overwriting an Ubuntu Delta. This page doesn't.

Doesn't close bugs: LP Bugs fixed in the upload aren't closed (we'll do this in the command line API client). Even if they were, we'd need to catch all bugs fixed by intermediate uploads too (LP: #827576)

What I'd like to see for non archive-admins:
* An individual sync button for each package (in the folded area)
* A box in the source package's overview page with upstream versions and a sync button / link to a page where it can be synced.
* Confirmation when syncing, showing: package, old version, new version, changes (including intermediate versions).

[Colin Watson (cjwatson)]

I agree with everything Stefano has written here, and I don't think anything Ubuntu-specific would be required to rectify his concerns.

[Julian Edwards (julian-edwards)]

I'm pretty disappointed to see a bug like this after we went through a lot of user testing with uploaders and archive admins alike and nobody mentioned these concerns before. We will look at the user testing process and see how it can be improved.

I'll comment on a few specific parts before breaking this bug out into separate tasks, this is too-wide a bug to fix everything in one go.

{{{
Ease of making mistakes: The interface has no confirmation when making a single sync, or (I'd imagine, from the massive accidental sync) a mass sync. There's a lot of checkboxes on the screen, and one of them could have been accidentally checked.
}}}

I'll look into adding a confirmation page for this.

{{{
Our current sync request tool asks for additional confirmation when overwriting an Ubuntu Delta. This page doesn't.
}}}

The way that Ubuntu deltas are indicated is very Ubuntu-specific, I don't know of a way of handling this in a generic fashion. I think the only reasonable place to handle it is in the sync tool script that you guys have.

{{{
Doesn't close bugs: LP Bugs fixed in the upload aren't closed (we'll do this in the command line API client). Even if they were, we'd need to catch all bugs fixed by intermediate uploads too (LP: #827576)
}}}

Will the intermediate change log entries contain all the bugs you need to close?

I'll reply again shortly when I've filed individual bugs.

[Julian Edwards (julian-edwards)]

 * An individual sync button for each package (in the folded area)
Bug #830987

* A box in the source package's overview page with upstream versions and a sync button / link to a page where it can be synced.
Bug #830984

* Confirmation when syncing, showing: package, old version, new version, changes (including intermediate versions).
Bug #830982

[Julian Edwards (julian-edwards)]

Invalidated in favour of the other bugs.

[Stefano Rivera (stefanor)]

Julian: Thanks for splitting them out for me. Yeah I don't like being a party pooper, and I probably should have tried it on dogfood when it was announced, but practically I only found myself looking at it when it was generally available. It looks like a few bugs slipped through until then.

I was surprised how powerful the interface was, compared to, say, retrying a failed build, which requires finding all the build records, clicking retry button (one per-arch) and confirming each retry.
Retrying a build is not a dangerous operation. Worst case scenario, it'll fail again, only wasted buildd time.
Syncing a package from Debian is something that needs a lot more thought, reviewing of diffs and probably a test build. It's not something I'm going to do as a batch process.

OTOH, we've been waiting for this for ages, and it's fantastic to see it :)

[Stefano Rivera (stefanor)]

> The way that Ubuntu deltas are indicated is very Ubuntu-specific, I don't know of a way of handling this in a generic fashion.

A reasonable algorithm may be:
If the latest version number in the destination is not present in the source's history and this version number with "build\d+$" removed is, then there's a delta that should be checked.

> Will the intermediate change log entries contain all the bugs you need to close?

Not all of them, but if a Debian uploader wanted to close an Ubuntu bug with an upload, it should happen :)

[Scott Kitterman (kitterman)]

On Monday, August 22, 2011 08:41:35 AM you wrote:
> Not all of them, but if a Debian uploader wanted to close an Ubuntu bug
> with an upload, it should happen

I see this done reasonably frequently and do it myself, so it would be good
for this to work (and also AIUI a regression from the current method if it
doesn't).

[Julian Edwards (julian-edwards)]

On Monday 22 August 2011 13:41:35 you wrote:
> A reasonable algorithm may be:
> If the latest version number in the destination is not present in the
> source's history and this version number with "build\d+$" removed is, then
> there's a delta that should be checked.

build\d+$ is also Ubuntu specific :)

However, we *could* standardise this but we'd need a more general consensus
that this is what we want, in particular with the Linaro guys who will be
using this feature to make Ubuntu derivatives.

Note also that we have an outstanding bug about no-change rebuilds in LP which
might make this moot though.

> Not all of them, but if a Debian uploader wanted to close an Ubuntu bug
> with an upload, it should happen :)

Yup, we're working on that. I need to fix bug 827576 first.

Thanks for the continuing feedback and sorry if I sounded ungrateful earlier -
I'm not, you just caught me at a bad time. Everyone's getting a little burned
out from working on this feature for so long.

[Stefano Rivera (stefanor)]

> build\d+$ is also Ubuntu specific :)

Of course, I think the meaning is relatively unambiguous, but that
doesn't mean it should be used without agreement in the derivatives. And
yes, it could plausibly result in -1build1build1 :/

> Note also that we have an outstanding bug about no-change rebuilds in LP which
> might make this moot though.

I think it's also an option to ignore this issue, the Ubuntu developer
can confirm the sync when LP says "there's a delta: foo-1build1" because
they no it means no-change-rebuild.

> Yup, we're working on that. I need to fix bug 827576 first.

Agreed, thanks.

> Thanks for the continuing feedback and sorry if I sounded ungrateful earlier -
> I'm not, you just caught me at a bad time. Everyone's getting a little burned
> out from working on this feature for so long.

NP, I can understand the frustration :)

[Colin Watson (cjwatson)]

Julian, for the record: in the user testing session I had with you, I explicitly said that I thought there were going to be issues of Ubuntu-specific policy (such as Ubuntu deltas) that were hard to handle in the UI, and that I would rather we used an API script for this. Now, I know from a previous conversation that some of the notes from that meeting got lost, but please don't turn that into nobody having raised any of these issues before ...

[Julian Edwards (julian-edwards)]

On Wednesday 24 August 2011 13:19:10 you wrote:
> Julian, for the record: in the user testing session I had with you, I
> explicitly said that I thought there were going to be issues of Ubuntu-
> specific policy (such as Ubuntu deltas) that were hard to handle in the
> UI, and that I would rather we used an API script for this. Now, I know
> from a previous conversation that some of the notes from that meeting
> got lost, but please don't turn that into nobody having raised any of
> these issues before ...

Colin, I'm aware of that and I've not lost any notes from that session. It's
based on your direct feedback here that we have consciously not filled all
those use cases in the web UI and made (or are making) the API able to do what
you need.

During all the user testing sessions nobody mentioned any of the specific
issues raised *in this bug*. Here's a raw dump of your own feedback from my
not-lost notes:

 * found new pages quickly
 * doesn't know what resolved packages means
 * didnt get blacklisting right
 * cares more about stuff that needs syncing
 * wants to hover on the ellipsed comment to see it all
 * blacklist terminology is confusing, rename it.
 * need to import ubuntu sync blacklist
 * archive admins/uploaders only to blacklist
 * fix popup text for "read more about syncing"
 * search by packagesets and "last uploaded"
 * search box text should not "show....matching" as it's not a glob search
 * show child packageset
 * packagenames should be green link
 * blacklist "this version" is not accurate enough, "these versions" maybe
 * is only largely interested in packages requiring action, syncing up or
merging down
 * think about splitting +localpackagediffs into 2 pages or adding search
option to separate changes only in child or parent
 * get rid of update button
 * higher versions in child distro are not syncable

In feedback from other people I have listed:
 * Need a "search all" regardless of blacklisting status
 * Missing navigation links at bottom of page
 * Latest comment column is not updating
 * Make link to parent clickable
 * debdiff with just debian dir would be nice
 * make "last common version" clickable
 * search by component/person/packageset
 * in-page sorting
 * rogue line in expanded section

(We have addressed nearly all of these points)

So, can you tell me where this particular bug's issues were raised in user
testing before? I'd like to know so we can fix our process, not point
fingers.

[Stefano Rivera (stefanor)]

> > Not all of them, but if a Debian uploader wanted to close an Ubuntu bug
> > with an upload, it should happen :)
> Yup, we're working on that. I need to fix bug 827576 first.

Filed bug 833736 for closing bugs on sync

[Iain Lane (laney)]

Why not just "if the latest version in the destination is not present in the source then ask for explicit confirmation (a checkbox) before overwriting it"? Yes, you'd get false positives with buildX versions but we can probably live with that if the alternative is mistakes. A "yes, I'm sure, please overwrite all of these changes" button which toggles all of the checkboxes when doing several syncs would make it less cumbersome.