[Debian-olpc-devel] review sugar-browse-activity

[Jonas Smedegaard]

Hi Ishan,

On Fri, Oct 22, 2010 at 12:42:41AM +0530, Ishan Bansal wrote:
>Sorry for the late response.

No problem at all: we are not in a hurry here (or at least I am not).


>1. Updated the browse from version '115' to '119'.

This is one of those changes that are difficult to roll back if done 
wrong.

I would like to do that myself for the packages that (primarily) I have 
worked on, including the Browse activity.

Please gain some experience in other activities that you start packaging 
yourself.

Hmmm - or how do you or others think about this?  Am I acting childish 
here, not letting others playing with "my" toys when really we are all 
in it together?



>2. Bumped the standard version to '3.9.1.0'.

Only the first three parts should be used when declaring standards 
version: the fourth is used only for editorial changes like grammatical 
errors and typos.

Also, did you in fact verify that the packaging comply with that version 
of Debian Policy?

If you are uncertain about that, then it is better to _not_ bump 
standards version!  What lintian warns about is not only that the number 
is low, but that the packaging should be generally examined and verified 
that it follows an up-to-date version of Debian Policy.



>3. Commented the patch "1002_revert_gnome_hack.patch" in the series 
>file as it gave http://paste.ubuntu.com/517605/ error in build.

That error only demonstrates that the patch do not fit exactly to the 
source it is tried to be applied against.  No surprise, since you 
updated the source: When updating upstream source, each and every patch 
needs to be examined if they need refreshing or maybe rewriting or maybe 
is obsoleted or conflicts with upstream changes.


>I have tested the changes locally and it seems to be working fine.

Great that you tested.  But when disabling a patch you should probably 
test specifically if you did not cause a regression.  Did you test that?


So to summarize:

  1. I prefer to do this one myself (please do argue against that if you 
     find my reasoning bad!)
  2. Stylistic nitpicking + potential underlying problem.  Possibly 
     nothing wrong with the change itself...
  3. I suspect (from your reasoning) that you did not understand the 
     purpose of the patch, and therefore not either the consequence of
     dropping it.

While writing this email, I discovered that I had 6 commits lying in my 
local git for Browse which I had not released.  I will do that now, and 
also bump to latest release.  I probably (depending on th actual 
upstream changes) will _not_ disable the patch, as I see it as relevant 
(that's why I added it in the first place - but I will add DEP3 headers 
to help clarify what _is_ its purpose, to help you in the future to 
understand my work :-)


Thanks for your proposed changes.  Please keep them coming!


Kind regards,

  - Jonas

-- 
  * Jonas Smedegaard - idealist & Internet-arkitekt
  * Tlf.: +45 40843136  Website: http://dr.jones.dk/

  [x] quote me freely  [ ] ask before reusing  [ ] keep private
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/debian-olpc-devel/attachments/20101021/8a38b78a/attachment.pgp>