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

Enable using these modules with version constraints #28

Merged
merged 2 commits into from
Sep 19, 2023
Merged

Conversation

mogul
Copy link
Contributor

@mogul mogul commented Sep 14, 2023

Adds a semver module that will calculate a tag version from this repository using NPM-style version constraints.

This does not update the main README's examples to use this new semver method; we should probably discuss how best to do that before merging, hence the draft state of the PR.

This does not update the main README's examples to use semver; we should
probably discuss how best to do that.
Copy link
Contributor

@rahearn rahearn left a comment

Choose a reason for hiding this comment

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

I like this idea, and I think for the main README we could probably do what you're doing in semver/README.md with the locals block either saying "FILL IN CONSTRAINTS" or doing something like ~>0.7 that will get all updates up to 1.0 and we can keep that more or less constant until we have breaking changes.

@mogul
Copy link
Contributor Author

mogul commented Sep 15, 2023

So each stanza in the README would look something like this? (Note I'm terrible at understanding version constraint syntax, so I'm kind of guessing based on the sparse examples here. And note this module uses NPM's version constraints, not Terraform's!)

module "latest-non-breaking-version" {
  source             = "github.com/18f/terraform-cloudgov//semver"
  version_constraint = "~0.7"
}

module "database" {
  source = "github.com/18f/terraform-cloudgov//database?ref=v${module.latest-non-breaking-version.target_version}"
  # [...]
}

(I also think it might be a good time to start moving those stanzas into a README.md in each subdirectory.)

@mogul
Copy link
Contributor Author

mogul commented Sep 15, 2023

Oh oh, wait, you meant actually using the exact same pattern with the for_each at the top of the main README.md! OK, let me add a commit for that.

@rahearn
Copy link
Contributor

rahearn commented Sep 15, 2023

Oh oh, wait, you meant actually using the exact same pattern with the for_each at the top of the main README.md! OK, let me add a commit for that.

I did mean that, but also I'm good with either way

@mogul mogul marked this pull request as ready for review September 18, 2023 19:31
@mogul mogul requested a review from rahearn September 18, 2023 19:31
@mogul
Copy link
Contributor Author

mogul commented Sep 18, 2023

@rahearn or @pauldoomgov can you approve this one? I'd like to merge it in support of moving stuff out of the FAC repo into here.

Copy link
Contributor

@rahearn rahearn left a comment

Choose a reason for hiding this comment

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

Love that this is a purely optional quality of life add-on

@mogul mogul merged commit 0b02d60 into main Sep 19, 2023
@mogul mogul deleted the add-semver-module branch September 19, 2023 01:57
@mogul
Copy link
Contributor Author

mogul commented Mar 1, 2024

Welp, uh, bad news... You cannot use this module's output as the source value for another module, because that would be an expression. Doh! Expect a PR rolling out the documentation changes.

mogul added a commit that referenced this pull request Mar 1, 2024
This reverts commit 0b02d60, reversing
changes made to 7db01d5.

# Conflicts:
#	README.md
#	semver/tests/default/default.tf
@mogul mogul mentioned this pull request Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants