[Merge] ~hyask/ubuntu-dev-tools:skia/i-b-f-d_multipart into ubuntu-dev-tools:main

Benjamin Drung mp+468091 at code.launchpad.net
Thu Oct 31 12:59:25 UTC 2024


I would have expected that the splitting the big function into smaller one to be in the commit before the bug fix.

Reviewing the refactoring making easier: each newly added function could be in a separate function.

Diff comments:

> diff --git a/import-bug-from-debian b/import-bug-from-debian
> index 5d0fd7f..c9dda73 100755
> --- a/import-bug-from-debian
> +++ b/import-bug-from-debian
> @@ -137,24 +173,71 @@ def main():
>          Logger.debug("Subject: %s", subject)
>          Logger.debug("Description: ")
>          Logger.debug(description)
> -
> -        if options.dry_run:
> +        for attachment in attachments:

You could simplify that to:

for i, attachment in enumerate(attachments):

> +            i, attachment = attachment
> +            Logger.debug("Attachment #%s (%s)", i, attachment.get_filename() or "inline")
> +            Logger.debug("Content:")
> +            if attachment.get_content_type() == "text/plain":
> +                content = attachment.get_content()
> +                if len(content) > 2000:

Please make 2000 a global constant.

> +                    content = content[:2000] + " [attachment cropped after 2000 characters...]"
> +                Logger.debug(content)
> +            else:
> +                Logger.debug("[data]")
> +
> +        if dry_run:
>              Logger.info("Dry-Run: not creating Ubuntu bug.")
>              continue
>  
>          u_bug = launchpad.bugs.createBug(target=target, title=subject, description=description)
> +        for i, attachment in attachments:
> +            name = f"#{i}-{attachment.get_filename() or "inline"}"
> +            content = attachment.get_content()
> +            if isinstance(content, str):
> +                # Launchpad only wants bytes
> +                content = content.encode()
> +            u_bug.addAttachment(
> +                filename=name,
> +                data=content,
> +                comment=f"Imported from Debian bug http://bugs.debian.org/{bug_num}",
> +            )
>          d_sp = debian.getSourcePackage(name=package)
> -        if d_sp is None and options.package:
> -            d_sp = debian.getSourcePackage(name=options.package)
> +        if d_sp is None and package:
> +            d_sp = debian.getSourcePackage(name=package)
>          d_task = u_bug.addTask(target=d_sp)
>          d_watch = u_bug.addWatch(remote_bug=bug_num, bug_tracker=lp_debbugs)
>          d_task.bug_watch = d_watch
>          d_task.lp_save()
>          Logger.info("Opened %s", u_bug.web_link)
> -        if not options.browserless:
> +        if not browserless:
>              webbrowser.open(u_bug.web_link)
>  
> -    if err:
> +    return err
> +
> +
> +def main():
> +    options = parse_args()
> +
> +    config = UDTConfig(options.no_conf)
> +    if options.lpinstance is None:
> +        options.lpinstance = config.get_value("LPINSTANCE")
> +
> +    if options.dry_run:
> +        launchpad = Launchpad.login_anonymously("ubuntu-dev-tools")
> +        options.verbose = True
> +    else:
> +        launchpad = Launchpad.login_with("ubuntu-dev-tools", options.lpinstance)
> +
> +    if options.verbose:
> +        Logger.setLevel(logging.DEBUG)
> +
> +    bugs = debianbts.get_status(get_bug_numbers(options.bugs))
> +
> +    if not bugs:
> +        Logger.error("Cannot find any of the listed bugs")
> +        sys.exit(1)
> +
> +    if process_bugs(bugs, launchpad, options.package, options.dry_run, options.browserless):
>          sys.exit(1)
>  
>  


-- 
https://code.launchpad.net/~hyask/ubuntu-dev-tools/+git/ubuntu-dev-tools/+merge/468091
Your team Ubuntu Development Team is subscribed to branch ubuntu-dev-tools:main.




More information about the Ubuntu-reviews mailing list