[opensuse-buildservice] Re: [PATCH] - add 'requestbugownership' command for setting the bugowner via request
Just as a side note, this is not the first example, but one of those commits that made a total rewrite necessary. By quality, the last ones by Ludwig and Vincent aren't any better either. Ideally, fixing sth. shouldn't be only about adding lines but about re-factoring lines. Otherwise the next commiter might be tempted to add his personal 'osc scratchmyback' command w/o communication too. @suse-tux: For the osc rewrite, I strongly urge you to enforce peer-reviews for commits. Or even better, don't let anyone commit directly but let them file merge requests, which can be reviewed and commented appropriatly. Otherwise the new osc will end up exactly like the current one, as an unmaintainable pile of crap code. We also should think about adapting a similar policy for OBS code, but I fear this would break quite some odd habbits... On Wednesday 01 June 2011 09:28:33 OBS osc wrote:
From: Adrian Schröter
--- NEWS | 1 + osc/commandline.py | 24 +++++++++++++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/NEWS b/NEWS index f9bb6a6..3002937 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ 0.132 - rdelete and undelete command requesting now a comment + - add 'requestbugownership' command for setting the bugowner via request # # Features which requires OBS 2.3 # diff --git a/osc/commandline.py b/osc/commandline.py index 2090d7b..04c091f 100644 --- a/osc/commandline.py +++ b/osc/commandline.py @@ -1533,18 +1533,23 @@ Please submit there instead, or use --nodevelproject to force direct submission.
@cmdln.option('-m', '--message', metavar='TEXT', help='specify message TEXT') - @cmdln.option('-r', '--role', metavar='role', default='maintainer', + @cmdln.option('-r', '--role', metavar='role', help='specify user role (default: maintainer)') + @cmdln.alias("reqbugownership") + @cmdln.alias("requestbugownership") @cmdln.alias("reqmaintainership") @cmdln.alias("reqms") + @cmdln.alias("reqbs") def do_requestmaintainership(self, subcmd, opts, *args): - """${cmd_name}: requests to add user as maintainer + """${cmd_name}: requests to add user as maintainer or bugowner
usage: osc requestmaintainership # for current user in checked out package osc requestmaintainership USER # for specified user in checked out package osc requestmaintainership PROJECT PACKAGE # for current user osc requestmaintainership PROJECT PACKAGE USER # request for specified user + + osc requestbugownership ... # accepts same parameters but uses bugowner role
${cmd_option_list} """ @@ -1570,14 +1575,23 @@ Please submit there instead, or use --nodevelproject to force direct submission. else: raise oscerr.WrongArgs('Wrong number of arguments.')
- if not opts.role in ('maintainer', 'bugowner'): + role = 'maintainer' + if subcmd in ( 'reqbugownership', 'requestbugownership', 'reqbs' ): + role = 'bugowner' + if opts.role: + role = opts.role + if not role in ('maintainer', 'bugowner'): raise oscerr.WrongOptions('invalid \'--role\': either specify \'maintainer\' or \'bugowner\'') if not opts.message: opts.message = edit_message()
r = Request() - r.add_action('add_role', tgt_project=project, tgt_package=package, - person_name=user, person_role=opts.role) + if role == 'bugowner': + r.add_action('set_bugowner', tgt_project=project, tgt_package=package, + person_name=user) + else: + r.add_action('add_role', tgt_project=project, tgt_package=package, + person_name=user, person_role=role) r.description = cgi.escape(opts.message or '') r.create(apiurl) print r.reqid
-- With kind regards, Sascha Peilicke SUSE Linux Products GmbH - GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer Maxfeldstraße 5, 90409 Nuernberg, Germany - HRB 16746 (AG Nuernberg)
Am Wednesday, 1. June 2011, 10:05:51 schrieb Sascha Peilicke:
Just as a side note, this is not the first example, but one of those commits that made a total rewrite necessary. By quality, the last ones by Ludwig and Vincent aren't any better either. Ideally, fixing sth. shouldn't be only about adding lines but about re-factoring lines. Otherwise the next commiter might be tempted to add his personal 'osc scratchmyback' command w/o communication too.
Don't complain without telling with which code this should be merged/refactored with ;) And current code did just the wrong thing leading to invalid data (multiple bugowner entries) by default. -- Adrian Schroeter SUSE Linux Products GmbH email: adrian@suse.de
On Wednesday 01 June 2011 10:18:49 Adrian Schröter wrote:
Am Wednesday, 1. June 2011, 10:05:51 schrieb Sascha Peilicke:
Just as a side note, this is not the first example, but one of those commits that made a total rewrite necessary. By quality, the last ones by Ludwig and Vincent aren't any better either. Ideally, fixing sth. shouldn't be only about adding lines but about re-factoring lines. Otherwise the next commiter might be tempted to add his personal 'osc scratchmyback' command w/o communication too.
Don't complain without telling with which code this should be merged/refactored with ;) And current code did just the wrong thing leading to invalid data (multiple bugowner entries) by default.
Adrian, it's about adding yet some more osc commands without fixing those that are already existant. It's about bloating the code and copy pasting stuff around. Lastly, it's about commiting quick'n'dirty first, maybe communicate afterwards and if in doubt tell, it 'can be fixed later'. The latter is a tale told ever so often with regards to Build Service code. We should start thinking about how to fix this behavior now. The OBS may have been a pet-project by some, but now it is used by lots. If we want to keep it like this, we have to deliver quality stuff. And, if you insist we're an open project, we should act as one. Therefore, I deliberately keep this discussion public. -- With kind regards, Sascha Peilicke SUSE Linux Products GmbH - GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer Maxfeldstraße 5, 90409 Nuernberg, Germany - HRB 16746 (AG Nuernberg)
Am Wednesday, 1. June 2011, 10:25:35 schrieb Sascha Peilicke:
On Wednesday 01 June 2011 10:18:49 Adrian Schröter wrote:
Am Wednesday, 1. June 2011, 10:05:51 schrieb Sascha Peilicke:
Just as a side note, this is not the first example, but one of those commits that made a total rewrite necessary. By quality, the last ones by Ludwig and Vincent aren't any better either. Ideally, fixing sth. shouldn't be only about adding lines but about re-factoring lines. Otherwise the next commiter might be tempted to add his personal 'osc scratchmyback' command w/o communication too.
Don't complain without telling with which code this should be merged/refactored with ;) And current code did just the wrong thing leading to invalid data (multiple bugowner entries) by default.
Adrian, it's about adding yet some more osc commands without fixing those that are already existant. It's about bloating the code and copy pasting stuff around. Lastly, it's about commiting quick'n'dirty first, maybe communicate afterwards and if in doubt tell, it 'can be fixed later'. The latter is a tale told ever so often with regards to Build Service code.
We should start thinking about how to fix this behavior now. The OBS may have been a pet-project by some, but now it is used by lots. If we want to keep it like this, we have to deliver quality stuff.
And, if you insist we're an open project, we should act as one. Therefore, I deliberately keep this discussion public.
I completly agree that we need a refactoring. However, I saw this more as a bug in first place which should be fixed ASAP to avoid more invalid data in our servers. A peer review, like we do right now makes of course sense. I would not release this osc as stable release before talking with TuX (maybe better via the mailing list the next time). -- Adrian Schroeter SUSE Linux Products GmbH email: adrian@suse.de
On 01/06/11 09:05, Sascha Peilicke wrote:
Otherwise the next commiter might be tempted to add his personal 'osc scratchmyback' command w/o communication too.
I agree.
@suse-tux: For the osc rewrite, I strongly urge you to enforce peer-reviews for commits. Or even better, don't let anyone commit directly but let them file merge requests, which can be reviewed and commented appropriatly. Otherwise the new osc will end up exactly like the current one, as an unmaintainable pile of crap code.
As a user of OBS who has multiple teams who want different workflows I'm very concerned about hardcoding workflow into osc.
We also should think about adapting a similar policy for OBS code, but I fear this would break quite some odd habbits...
This is a reasonable thing to want to do. In Nokia and MeeGo we've been working on BOSS to abstract the workflow out of OBS and into a small and focused workflow system. It allows users of different OBS installations (or even different project areas on the same OBS) to use discrete workflows that interact with other systems like bugzilla or Hermes. David -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
participants (3)
-
Adrian Schröter
-
David Greaves
-
Sascha Peilicke