PR#330: Add new option --name and --namespace to deprecate --module-name -

rpkg

#330 Add new option --name and --namespace to deprecate --module-name

Merged Jun 23, 2018 by cqi. Opened Jun 14, 2018 by cqi.

cqi/rpkg new-option-name-namespace into master

cqi commented Jun 14, 2018

--module-name can still be used but deprecation message is printed. As a result, a use case of --module-name, e.g. fedpkg --module-name modules/somemodule request-repo, could be replaced with fedpkg --name somemodule --namespace modules request-repo.

In addition, if repo_namespaces is configured, for example in fedpkg.conf, value of --namespace will be validated.

Fixes #301

1 new commit added

Massive replacement of module

Jun 14, 2018

cqi commented Jun 14, 2018

Pretty please pagure-ci rebuild

lsedlar commented Jun 14, 2018

Typo: omtted -> omitted

lsedlar commented Jun 14, 2018

This seems inaccurate. It's used not only for cloning, but in other operations as well (like verrel). Can it reuse the original help from --module-name? Something like Override repository name. Use --namespace option to change namespace. If not specified, name is discovered from Git push URL or Git URL (last part of path with .git extension removed) or from Name macro in spec file, in that order.

lsedlar commented Jun 14, 2018

Can the error be changed to missing --name: using --namespace requires --name option

lsedlar commented Jun 14, 2018

Mostly looks good to me. I have a concern about the deprecation warnings in module_name property. Those will be printed to users, but they don't really have any way to fix that. I'm sure some people will be confused by that, and there will be bugs opened about it.

cqi commented Jun 15, 2018

It's inaccurate indeed. I like your version, it's clear enough. Thanks :)

cqi commented Jun 15, 2018

Mostly looks good to me. I have a concern about the deprecation warnings in module_name property. Those will be printed to users, but they don't really have any way to fix that. I'm sure some people will be confused by that, and there will be bugs opened about it.

Deprecation of --module-name and property Commands.module_name will be mentioned in released notes, which is for developers. Printing deprecation warnings would be a last way to notify the deprecation. How about log the message in property module_name in debug level? Or we can just keep them and user is able to run with -q option.

1 new commit added

Fix typo and reword option help and deprecation message

Jun 15, 2018

lsedlar commented Jun 15, 2018

These two conditions will crash if clone config is not set. Running fedpkg tests against this PR runs into it.

lsedlar commented Jun 15, 2018

Rhpkg tests are not passing against this PR, but that looks like some mock going wrong in the test suite itself.

cqi commented Jun 15, 2018

Yeah, rhpkg needs to update in order to work with this patch. There is a PR for fedpkg for that as well.

lsedlar commented Jun 20, 2018

Mostly looks good to me. The last issue is the inline comment in cli.py about clone config. As is, this change will require all downstreams to define some clone configuration. Can the condition be updated to work even when clone_config is None?

1 new commit added