#help (2020-09)

Where to get help about getting help!

2020-09-18

2020-09-16

Iouns avatar
Iouns

Hello there, I don’t know where to ask a github related question. I did some PR on a repo and just want to know if there is something wrong with it or if I just need to wait someone have some time to review it.

Iouns avatar
Iouns
Add variable label_order. by Iouns · Pull Request #52 · cloudposse/terraform-aws-s3-bucket

what Add variable label_order. why More flexibility to handle resources naming. references N/A

Iouns avatar
Iouns

First time I try to PR your modules, so I’m not sure to do things correctly

Iouns avatar
Iouns

Ok… I think I found out why hmm

Gowiem avatar
Gowiem

@ Thanks for the contribution. We are trying to move all modules over to use terraform-null-label’s new context approach. You can see an example here: https://github.com/cloudposse/terraform-aws-eks-cluster/pull/75

Convert to context.tf, allow AWS 3.0 provider by Nuru · Pull Request #75 · cloudposse/terraform-aws-eks-cluster

what Allow AWS provider version 3.0 Convert to context.tf, update chatops, add auto-release why New features, specifically launch templates Standardize notes Closes #74

Iouns avatar
Iouns

Yeah, I did realized that digging through the k8s module

Iouns avatar
Iouns

It’s a much more complex work, I’ll try to make another PR with this implementation

Iouns avatar
Iouns

Thank you for your guidance

Iouns avatar
Iouns

and for the really cool modules

1
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

@Andriy Knysh (Cloud Posse) can also help with this

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

sure

1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

@ what is your PR? I can take a look and we’ll fix it

Iouns avatar
Iouns

Unfortunatly, I need to rework it a little bit. I’ll get back to you as soon as I can

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

ok, for [contxt.tf](http://contxt\.tf) related PR (also to update Terratest to go modules and other stuff), see these examples

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
Update to context. Update Terratest to Golang modules. Allow TF 0.13 by aknysh · Pull Request #55 · cloudposse/terraform-aws-efs

what Update to context.tf Update Terratest to Golang modules Allow TF 0.13 Add Security Group ingress for CIDR blocks why Standardization and interoperability Keep the module up to date related…

Iouns avatar
Iouns

great ty

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
Update to `context.tf` by aknysh · Pull Request #14 · cloudposse/terraform-aws-ses

what Update to context.tf why Standardization and interoperability Keep the module up to date

Iouns avatar
Iouns

ok great, this sounds not so hard to PR, I’ll try to do that by the end the end of the week

Iouns avatar
Iouns

Pretty cool to you to lead me to the correct contribution path like that, ty guys

:--1:1
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

when you download the repo to your computer, cd to it and then run these commands (besides making changes to the code):

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
make init
make github/init
make readme
1
Iouns avatar
Iouns

One last thing, do you have some github issue/discussion which explain the rationale for moving to context.tf ?

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
  1. to standardize on the common fields like namespace, stage, environment, name, attributes, tags, enabled
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
  1. make modules invocation simpler by just providing the context instead of (inconsistently) repeating all the variables
Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

Iouns avatar
Iouns

Okok. ty

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

here are almost complete steps to convert to context and go modules:

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)
make init
make github/init
cd test/src/
go mod init [github.com/cloudposse/$(cd](http://github\.com/cloudposse/\$\(cd) ../../ && basename `pwd`)
go mod tidy
git rm Gopkg.*
git add go.*
<copy context.tf into the module and into the examples/complete>
<remove redundant variables (namespace, stage, environment, name, attributes, tags, enabled) from variables.tf in the module and examples/complete>
<remove redundant `terraform-null-label` modules from the module, use the label from context.tf whenever possible. If you need to add attributes, use a new label invocation and provide the context>
<update the module code and the example code to use context, update the Terratest `go` code to use random attributes for testing>
make readme
:--1:1
Iouns avatar
Iouns

Perfect, I’ll try that then. thank you very much

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

yes, aws provider v3 does not support region attribute

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

we have updated terraform-aws-s3-bucket module for that

Iouns avatar
Iouns

Yes I saw that PR

Iouns avatar
Iouns

I hope to finish this tomorrow

:--1:1
Iouns avatar
Iouns

Iouns avatar
Iouns

Here the first one: https://github.com/cloudposse/terraform-aws-iam-s3-user/pull/22 Once reviwed, merged and tagged I’ll finish the terraform-aws-s3-bucket

Andriy Knysh (Cloud Posse) avatar
Andriy Knysh (Cloud Posse)

looks good, thanks, left a comment

Iouns avatar
Iouns

Yeah I’m on it

Iouns avatar
Iouns

done

Iouns avatar
Iouns
Update to context by Iouns · Pull Request #53 · cloudposse/terraform-aws-s3-bucket

what Update to context. why N/A references Require cloudposse/terraform-aws-iam-s3-user#22 to be merged.

Iouns avatar
Iouns
Update to context. by Iouns · Pull Request #54 · cloudposse/terraform-aws-s3-bucket

Update to context what Update to context. Terraform provider v3 compatibility why N/A references Dependency on cloudposse/terraform-aws-iam-s3-user#22

    keyboard_arrow_up