Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Module names should be singular #10

Closed
abadger opened this issue Apr 19, 2016 · 14 comments
Closed

Proposal: Module names should be singular #10

abadger opened this issue Apr 19, 2016 · 14 comments
Assignees

Comments

@abadger
Copy link
Contributor

abadger commented Apr 19, 2016

Proposal: Module names should be singular

Author: Toshio Kuratomi @abadger

Date: 2016/04/19

  • Status: New
  • Proposal type: module design
  • Targeted Release: 2.1
  • Associated PR:
  • Estimated time to implement: 1 hour

Motivation

We want to make it easier for users to remember the names of modules.

Problems

it can be hard to remember names of modules when some of them are plural and some are singular. It gets even harder when modules are multiword: Was that module named vmware_vmkernels_ip_config.py, vmware_vmkernel_ip_configs.py, vmware_vmkernels_ip_configs.py, or vmware_vmkernel_ip_config.py?

It can also be hard to remember a name when there's inconsistency inside of a group of modules. This can be made even harder when a module belongs to several groups. We might have docker_images.py and therefore someone might look for docker_files.py. A different person might be using the file.py module and therefore look for docker_file.py.

Solution proposal

  • All new modules will be named using singular noun forms. Using the examples above we'd have:
    • vmware_vmkernel_ip_config.py
    • docker_image.py
    • docker_file.py
    • file.py
  • Existing modules which use plural should get aliases to singular form.
  • New modules may get aliases to plural form if it is deemed confusing (for instance, numerous existing modules in a similar category used the plural form before we instituted this policy). For example:
    • plan9_file.py may have an alias of _plan9_files.py if many existing modules end with the plural, _files, suffix.
  • The exception to this rule is when the plural form is part of a proper noun. Names of web services, names of command line tools (not their options), and companies or products fall under this. We also consider Ansible's Facts modules to be covered by this exception. Some examples of modules under this exception:
    • profitbricks.py (ProfitBricks is a cloud computing company)
    • rax_facts.py (this is a facts module)
    • iptables.py (iptables is both the name of a linux kernel subsystem and the command line tool for managing it)

Documentation

We should add documentation of this requirement to the Module Checklist in docsite/rst/developing_modules.rst

Alternatives

  • We could pluralize everything. Reasons not to use this alternative were: plural rules vary and so non-native speakers would have a harder time determining the module name, most modules currently use singular so it would affect far more modules than changing to singular.
  • We could have more exceptions for:
    • Command line tool subcommands. This was decided against as modules don't absolutely parallel the command line tool, it doesn't help the case where an upstream hasn't normalized their own arguments, and it doesn't help when a module falls into multiple categories. (However, aliases are appropriate in this situation).
@willthames
Copy link

How many modules are affected by this @abadger ?

My analysis says:

  • os_subnets_facts
  • os_networks_facts
  • rax_files_objects
  • os_server_actions
  • postgresql_privs
  • rax_cbs_attachments
  • rax_clb_nodes
  • rax_files
  • rax_files_objects
  • mysql_variables
  • cl_ports
  • dpkg_selections
  • macports
  • github_hooks
  • osx_defaults
  • pam_limits
  • win_updates
  • logentries

@bcoca
Copy link
Member

bcoca commented Apr 22, 2016

i can see several exceptions in that list, mostly cause of common names or tools 'logentries' is a company for example

@abadger
Copy link
Contributor Author

abadger commented Apr 23, 2016

Yeah, there shouldn't be too many current modules affected by this. As bcoca notes, logentries is a company, macports is the name of a project, pam_limits is the proper name of a PAM module. Here's the quick list of current modules I found that should get singular aliases:

  • cl_ports.py
  • dpkg_selections.py
  • ec2_vpc_dhcp_options.py
  • github_hooks.py
  • include_vars.py
  • mysql_variables.py
  • os_networks_facts.py
  • os_server_actions.py
  • os_subnets_facts.py
  • osx_defaults.py
  • postgresql_privs.py
  • rax_cbs_attachments.py
  • rax_clb_nodes.py
  • rax_files.py
  • rax_files_objects.py
  • win_updates.py

This was brought up in a new modules meeting where it was questioned whether a new module should take plural or singular form.

@bcoca
Copy link
Member

bcoca commented Jan 19, 2017

  • _facts, its a 'ansible standard' name for this type of plugin
  • dpkg_selections, win_updates, osx_defaults are based on the name of tool

@abadger
Copy link
Contributor Author

abadger commented Jan 19, 2017

Sorry I left this so long... here's a new list. Please propose things to remove from the list:

  • ./cloud/amazon/ec2_vpc_dhcp_options.py
  • ./cloud/rackspace/rax_cbs_attachments.py
  • ./cloud/rackspace/rax_files.py
  • ./cloud/rackspace/rax_clb_nodes.py
  • ./cloud/rackspace/rax_files_objects.py
  • ./cloud/ovirt/ovirt_nics.py
  • ./cloud/ovirt/ovirt_templates.py
  • ./cloud/ovirt/ovirt_mac_pools.py
  • ./cloud/ovirt/ovirt_hosts.py
  • ./cloud/ovirt/ovirt_external_providers.py
  • ./cloud/ovirt/ovirt_clusters.py
  • ./cloud/ovirt/ovirt_vmpools.py
  • ./cloud/ovirt/ovirt_disks.py
  • ./cloud/ovirt/ovirt_groups.py
  • ./cloud/ovirt/ovirt_storage_domains.py
  • ./cloud/ovirt/ovirt_host_networks.py
  • ./cloud/ovirt/ovirt_vms.py
  • ./cloud/ovirt/ovirt_permissions.py
  • ./cloud/ovirt/ovirt_tags.py
  • ./cloud/ovirt/ovirt_datacenters.py
  • ./cloud/ovirt/ovirt_quotas.py
  • ./cloud/ovirt/ovirt_users.py
  • ./cloud/ovirt/ovirt_affinity_labels.py
  • ./cloud/ovirt/ovirt_networks.py
  • ./cloud/profitbricks/profitbricks_volume_attachments.py
  • ./cloud/openstack/os_server_actions.py
  • ./database/mysql/mysql_variables.py
  • ./database/postgresql/postgresql_privs.py
  • ./network/cumulus/cl_ports.py
  • ./network/nxos/nxos_snmp_traps.py
  • ./network/nxos/nxos_ntp_options.py
  • ./source_control/github_hooks.py
    * ./monitoring/logentries.py logentries is a proper noun
  • ./storage/netapp/netapp_e_snapshot_images.py
    * ./utilities/logic/include_vars.py vars is a proper noun for an ansible concept (like facts)
    * ./utilities/logic/set_stats.py stats is a proper noun for an ansible concept (like facts)

@gundalow
Copy link
Contributor

This is now documented (cia ansible/ansible#21021)

It will appear on http://docs.ansible.com/ansible/dev_guide/developing_modules_documenting.html when docs are next pushed live

@samdoran samdoran self-assigned this Jul 17, 2017
@abadger
Copy link
Contributor Author

abadger commented Jul 21, 2017

How to rename a module with an alias to the old name:

  • Rename the module file to the new name.
  • Edit documentation to reflect the new name.
  • Make a symlink of _oldname.py that points to newname.py

@samdoran
Copy link

samdoran commented Jul 21, 2017

Current list of affected modules:

./cloud/amazon/ec2_vpc_dhcp_options.py
./cloud/openstack/os_server_actions.py
./cloud/ovirt/ovirt_affinity_groups.py
./cloud/ovirt/ovirt_affinity_labels.py
./cloud/ovirt/ovirt_clusters.py
./cloud/ovirt/ovirt_datacenters.py
./cloud/ovirt/ovirt_disks.py
./cloud/ovirt/ovirt_external_providers.py
./cloud/ovirt/ovirt_groups.py
./cloud/ovirt/ovirt_host_networks.py
./cloud/ovirt/ovirt_hosts.py
./cloud/ovirt/ovirt_mac_pools.py
./cloud/ovirt/ovirt_networks.py
./cloud/ovirt/ovirt_nics.py
./cloud/ovirt/ovirt_permissions.py
./cloud/ovirt/ovirt_quotas.py
./cloud/ovirt/ovirt_snapshots.py
./cloud/ovirt/ovirt_storage_connections.py
./cloud/ovirt/ovirt_storage_domains.py
./cloud/ovirt/ovirt_tags.py
./cloud/ovirt/ovirt_templates.py
./cloud/ovirt/ovirt_users.py
./cloud/ovirt/ovirt_vmpools.py
./cloud/ovirt/ovirt_vms.py
./cloud/pubnub/pubnub_blocks.py
./cloud/rackspace/rax_clb_nodes.py
./cloud/rackspace/rax_files.py
./cloud/rackspace/rax_files_objects.py
./database/mysql/mysql_variables.py
./database/postgresql/postgresql_privs.py
./database/proxysql/proxysql_backend_servers.py
./database/proxysql/proxysql_global_variables.py
./database/proxysql/proxysql_mysql_users.py
./database/proxysql/proxysql_query_rules.py
./database/proxysql/proxysql_replication_hostgroups.py
./network/avi/avi_cloudproperties.py
./network/avi/avi_controllerproperties.py
./network/avi/avi_seproperties.py
./network/cloudengine/ce_snmp_traps.py
./network/cumulus/_cl_ports.py
./network/nxos/nxos_ntp_options.py
./network/nxos/nxos_snmp_traps.py
./packaging/os/dpkg_selections.py
./source_control/github_hooks.py
./storage/netapp/netapp_e_snapshot_images.py
./storage/netapp/sf_check_connections.py
./windows/win_updates.py

Here is the regexp file I'm using as a filter:

(rds|sns|alias|stats|dns|facts|status|zfs|pam_limits|_os|known_hosts|jboss|address|fs|ucs)\.py
alternatives
aws_kms
capabilities
cbs
efs
include_vars
iptables
kubernetes
logentries
macports
nagios
osx_defaults
profitbricks
redis
serverless
xbps

Will submit an initial PR with a few changed shortly.

@bcoca
Copy link
Member

bcoca commented Jul 21, 2017

  • add check in module to see which name it was invoked with, to emit a deprecation notice when old one is used

samdoran added a commit to samdoran/ansible that referenced this issue Aug 14, 2017
samdoran added a commit to ansible/ansible that referenced this issue Aug 14, 2017
* First batch of modules renamed from plural to singular

Related to this proposal: ansible/proposals#10

* Emit rename deprication warning

* Update legacy-files.txt and skip.txt to reflect new names
@caphrim007
Copy link

@samdoran I dont mind changing the f5 module mentioned. it should not have any real existing users yet because it is a partial for a larger HA feature which is not fully implemented.

I do have an enhancement to the regex to be made though, there is an upcoming f5 module to deal with HTTPS related monitors, so that plurality will want to be noted (at least for f5) so that it is not flagged as it is a proper protocol.

@caphrim007
Copy link

I will send a PR for the f5 module mentioned

@samdoran
Copy link

@caphrim007 Thanks! Much appreciated.

@caphrim007
Copy link

caphrim007 commented Sep 14, 2017

@samdoran f5 can now be removed from the list

@bcoca
Copy link
Member

bcoca commented Apr 29, 2021

this was 'done' for existing modules, now with collections this becomes a optional rule to follow by their authors.

@bcoca bcoca closed this as completed Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants