cloudposse / terraform-null-label

Terraform Module to define a consistent naming convention by (namespace, stage, name, [attributes])
https://cloudposse.com/accelerate
Apache License 2.0
635 stars 317 forks source link

Stop using the "prehistoric" terraform naming for modules. #138

Closed gillg closed 1 year ago

gillg commented 1 year ago

To prevent bad practices by people downloading this file as a bootstrap, it will be a lot better to have a module name not meaningless.

what

why

gillg commented 1 year ago

@aknysh thanks for your very interesting answer !

I don't get your point about

Since context.tf is a mixin, this is supposed to mean not the null-label module, but the parent/root module itself where you use the context. So module.this.xxx means the module where you are now, and naming it this makes perfect sense

The module label is named this, so it's necessarly the null-label module itself ? In term of TF path it will be module.parent.module.this.xxxx so it's still meaningless when you read your plan or your state resources no ? You never know (except by bad experice) that this is related to the naming.

But I easily understand all your other points about complexity of migration. Maybe a complex but meainingfull step ? Maybe doable in multiple steps, but that should triggers zero change in all the TF stacks, and I would say there is no need for users to necessarly change their modules names... The can continue to use this if they prefer. That will just affect new deployments in theory no ?

Nuru commented 1 year ago

Closing as this is not something we will accept for the reasons outlined above.

TL;DR Changing the name will have a huge impact, and, as names are largely a matter of preference, does not guarantee any overall benefit. It is plausible that fewer people will like the new naming convention than the current one.

osterman commented 1 year ago

Avoid the name this which is meaningless, confusing, and was a kind of bad habbit in first ages of terraform.

I think some context may be missing. The terms this and self are pretty common in languages like C++, Python, and Perl. While Terraform does not support object-oriented programming, the terms were inspired by conventions in other languages.

Explicit the name of the module by naming so you can use it like module.naming.tags

I'm not familiar with the naming variable convention; I've not seen the convention used elsewhere, but please share documentation to the convention.

Keep in mind that terraform-null-label is the first module of its kind in the Terraform ecosystem, and now many other projects have adopted this "label" convention introduced by Cloud Posse. This module is now 8+ years old, and there are many things we would do differently now that Terraform has evolved; however, due to the limitations of Terraform at the time we first implemented it, and a general commitment to reducing breaking changes for our community, we've elected to reduce volatile changes to null label.

We have plans to evolve null-label without breaking backward compatibility, such as allowing for arbitrary taxonomies, but changing our naming convention of this is not something we've considered and would require significant discussion.