search

Azure / terraform-azure-modules

Azure verified modules for Terraform
MIT License
83 stars 32 forks source link

[Feature]: Codex version upgrade tests #16

Open matt-FFFFFF opened 1 year ago

matt-FFFFFF commented 1 year ago

Module Name

codex

/cc @grayzu

Description

In terms of feedback on codex:

https://github.com/Azure/terraform-azure-modules/blob/main/codex/test_code/version_upgrade_tests.md

This constraint prevents us introducing new best practice features using minor versions. Is this intended? With this, we are prevented from having the new best practice as the module

Example usage

No response

Other information

Please can we consider allowing changes in minor releases? As long as there is no breaking change requiring a code change in the module declaration (e.g. variable changing from list(object) to map(object))

lonegunmanb commented 1 year ago

Hi @matt-FFFFFF, apology for the late reply.

This constraint prevents us introducing new best practice features using minor versions. Is this intended?

Yes, it's intended. Any change that need our users pay attention to might frighten them. Due to the definition in semver:

MINOR version when you add functionality in a backward compatible manner

The users should feel safe to upgrade minor version without reading readme, if they haven't changed their code then nothing would change. I understand that introducing the latest best practices into a module is very important, but there's a trade-off that need to be made here. Since a Terraform module is related to the production infrastructure resources along with data, the codex emphasizes the importance of a stable and foreseeable code behavior.

This rule is inspired by phychological safety in Lean software development movement. People should feel safe to execute minor version upgrade, that could encourage them to adopt our latest improvement asap.

matt-FFFFFF commented 1 year ago

Hi @lonegunmanb

I agree with you definition of SEMVER.

However the I believe that the version upgrade check currently does not allow any changes in the plan when releasing a MINOR version. This prevents new non-breaking features being added.

See the text in the codex:

Then re-execute terraform init and terraform plan to see of there are changes. If the changes exist, that means the upgrade contains an "accident" breaking change.

This is incorrect as a MINOR change could introduce a new resource, therefore a change to the plan, in a non-breaking fashion.

Rather than simply looking for a change in the plan, we need to instead add logic to check for the following conditions: