Skip to content

add module naming convention #12

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add module naming convention #12

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,15 @@ proposes workarounds. The 0.14 version is not impacted by this issue.

We will maintain one mono repo per cloud provider. Three repos, representing AWS, Azure and GCP, as we kick off our efforts.

## Module naming convention
We will use [kebab-case](https://www.theserverside.com/definition/Kebab-case) when naming the modules
Copy link
Contributor

@jabielecki jabielecki May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be against. We have so many modules named with snake_case. Renaming all of them is a big change, so I'd wonder what strong arguments are there?

ls ../terraform-*/modules
../terraform-aws-vmseries-modules/modules:
asg  bootstrap  crosszone_failover  gwlb  panorama  transit_gateway  vmseries  vpc  vpc_endpoint  vpc_routes

../terraform-google-vmseries-modules/modules:
autoscale  bootstrap  gcp_bootstrap  iam_service_account  lb_http_ext_global  lb_tcp_external  lb_tcp_internal  panorama  vm  vmseries  vpc

When programming, there is a temptation to first do use the identical string:

module "security-group" { # <-- copy pasted from `source`
  source  = "terraform-aws-modules/security-group/aws"
  version = "4.0.0"
}

This either fails the linting rule after some time, or if uncaught, it leads to this painful experience:

id = module.security-group.security_group.sg_id

Subjectively I know the excerpts like the previous line hurt my productivity a lot. But I know it won't be the same for all of us.

(Introducing a long-existing inconsistency is worse. In usage, if a dev or user is always in doubt whether this time it's transit-gateway or transit_gateway, that sums up over all these people to quite a noticeable problem.)

Copy link
Contributor

@devsecfranklin devsecfranklin May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the name of the main module, rather than changing every variable name. I could be wrong though. Care to comment @migara ?


Examples
Copy link
Contributor

@damianfedeczko damianfedeczko May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Examples
Examples:

```
transit-gateway
gateway-loadbalancer
```

Copy link
Contributor

@damianfedeczko damianfedeczko May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Module structure:

├── README.md <- Documentation explaining the purpose of this mono repo
├── LICENSE.md <- Palo Alto Networks Script Software Agreement
├── modules/
Expand Down