rpkg

#349 Refactor build command
Merged by cqi. Opened by cqi.
cqi/rpkg refactor-build  into  master

Download 349.patch

This is part of the implementation of submitting builds from stream
branch when run fedpkg build. This refactor allows to override
cliClient.build without affecting the scratch-build and chain-build.

In addition to the refactor, tests are also added for command build,
scratch-build and chain-build.

In class cliClient.build, most of the code are moved into
a separate method cliClient._build, where is the right place to add
general code for all of the three build commands.

A global option --dry-run is added and build, scratch-build and
chainbuild could perform a dry run. This new option could also be used
in other commands rather than introducing new one for themselves.

Signed-off-by: Chenxiong Qi cqi@redhat.com

rebased onto 473955bb0a6660a44392939e09a9ea2def168f59

Required by https://pagure.io/fedpkg/pull-request/250

rebased onto 689e4e6fa04dad297f2e9f0e265ee9fa9de22544

In these if-else branches is duplicated code. Is it possible to eliminate it? But it is only optional, I can live without it.

Clean '#' characters from comments in next 2 rows.

Thank you for refactoring build method. It helps much with understanding the code.
I have no other comments.

This looks pretty nice!

Should _upload_srpm_for_build look at dry run option? RIght now it seems like it will always upload the file, even if the build task is not started.

I see you have improved formatting in a few places. Maybe using black could simplify that? I'm not saying to reformat everything, I just want to show an interesting tool.

In these if-else branches is duplicated code. Is it possible to eliminate it? But it is only optional, I can live without it.

Sorry, what is the duplicate code?

rebased onto 6a4ac54e1ff7dda542fca5d7d504cc3610fd8d78

I meant the code:

            if self.dry_run:
                self.log.info(
                    'DRY-RUN: kojisession.chainBuild(%s, %s, %r, priority=%s)',
                    chain, self.target, opts, priority)
                task_id = random.randint(1000, 2000)
            else:
                task_id = self.kojisession.chainBuild(
                    chain, self.target, opts, priority=priority)

but now I can see that these 2 similar branches are different: chainBuild vs build. So do not change it. I am sorry.

Clean '#' characters from comments in next 2 rows.

Fixed.

Should _upload_srpm_for_build look at dry run option? RIght now it seems like it will always upload the file, even if the build task is not started.

Fixed.

I see you have improved formatting in a few places. Maybe using black could simplify that? I'm not saying to reformat everything, I just want to show an interesting tool.

Will try :)

Confirmed. I am satisfied with the code.

:thumbsup:

rebased onto 2f5434c294a32815276798d689f1e3b8365e409e

rebased onto 7f681da08e330f699c139c6e651c8c2370c0a38f

Fixed a test TestBuildPackage.test_option_srpm_by_generate_srpm_from_repo, which didn't mock _run_command properly.

rebased onto f5c0197d54b67996c7462a014989f6b01427b30f

rebased onto c97e779383ac3360e207deb7b7be673450451626

Pull-Request has been merged by cqi
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.