A successful scratch build of the F29 package with this change (including this commit as a patch) can be viewed at: https://koji.fedoraproject.org/koji/taskinfo?taskID=30478441

See https://src.fedoraproject.org/fork/merlinm/rpms/rpkg/c/163e02ca9026311f522b942306b89dddbad17f5a?branch=retire_cmd for the patched package. I'll create a PR for the package update if this PR is approved.

The previous commit is in response to the discussion in https://pagure.io/fedpkg/pull-request/290. I'll be pushing a follow-up update to that PR soon.

https://pagure.io/fedpkg/pull-request/290 has been updated to make use of the configuration options provided by the latest commit to this PR.

Can you use the new test style with CliTestCase? Then, you don't need to repeat code inside _setup_repo and _fake_client.

@cqi Thank you for the suggestion. Hopefully my latest commit is what you had in mind.

I'm not sure this is a good design. There are more namespaces than just rpms and modules (both in Fedora and internally at Red Hat). Maybe a list of namespaces where retiring is allowed would work better? Though that makes it more difficult to allow it should a new namespace be added to Fedora.

One side note is that even if a downstream client disables the retiring, it's still possible for users to manually delete the files and add the notice. Thus no tool should rely on the retiring to not be enabled. From that point of view rather than saying it's not allowed by policy it could print a message saying that retiring in this namespace makes no sense and point to process that should be followed instead.

I'm not sure this is a good design. There are more namespaces than just rpms and modules (both in Fedora and internally at Red Hat). Maybe a list of namespaces where retiring is allowed would work better? Though that makes it more difficult to allow it should a new namespace be added to Fedora.

@lsedlar That's an excellent idea. If you're concerned about keeping up with new namespaces in the future, perhaps a list of namespaces where retiring is blocked would be more appropriate?

One side note is that even if a downstream client disables the retiring, it's still possible for users to manually delete the files and add the notice. Thus no tool should rely on the retiring to not be enabled. From that point of view rather than saying it's not allowed by policy it could print a message saying that retiring in this namespace makes no sense and point to process that should be followed instead.

Of course. The retire command is a convenience that does nothing a user couldn't do manually. It just automates the operation using standardized marker file names and content.

Regarding a message when things are blocked, are you suggesting a message saying something like "please check for documentation describing the proper policies and process"? Or did you want it to point to a specific URL?

Yeah, a blacklist is probably a better option than whitelist.

The message as you suggest it sounds fine to me. If rhpkg could modify it actually add the link, that would be convenient, but not required.

@lsedlar Apologies for the long delay. I have implemented the use of a configurable list of namespaces for which retirement is blocked--which includes 'rpms' by default. I also rebased my commits to the latest master branch. The corresponding fedpkg commit (https://pagure.io/fedpkg/pull-request/290) has been revised to remove 'rpms' from that list. rhpkg needs no code changes to pick up the retire sub-command from rpkg with RPM retirement disabled.

I just rebased to the latest master. I also added a commit with minor cleanup so python2.6 and flake8 test failures that I overlooked earlier are now passing.

Great. And now the time has come. I am going to merge it because there is a plan to release rpkg and fedpkg together. I am OK with the code. But I would like to ask you for squashing all your 5 commits into one. Or alternatively, close this pull request and open a new one with the final code. Would it be possible?
I propose the same approach at fedpkg PR https://pagure.io/fedpkg/pull-request/290

@onosek I rebased to the latest commit in master and squashed both this PR and https://pagure.io/fedpkg/pull-request/290. Thank you!

Looks good. Thanks!