Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

feat: use go-ipfs instead of js-ipfs #13

Merged
merged 14 commits into from
Apr 9, 2022

Conversation

smrz2001
Copy link
Collaborator

@smrz2001 smrz2001 commented Apr 5, 2022

No description provided.

@smrz2001 smrz2001 self-assigned this Apr 5, 2022
@linear
Copy link

linear bot commented Apr 5, 2022

@smrz2001 smrz2001 marked this pull request as ready for review April 5, 2022 23:27
Copy link
Contributor

@v-stickykeys v-stickykeys left a comment

Choose a reason for hiding this comment

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

Some questions

examples/ecs/main.tf Outdated Show resolved Hide resolved
examples/ecs/versions.tf Outdated Show resolved Hide resolved
modules/ecs/ipfs/locals.tf Show resolved Hide resolved
modules/ecs/ipfs/locals.tf Outdated Show resolved Hide resolved
modules/ecs/ipfs/main.tf Outdated Show resolved Hide resolved
modules/ecs/ipfs/storage.tf Outdated Show resolved Hide resolved
modules/ecs/ipfs/templates/container_definitions.json.tpl Outdated Show resolved Hide resolved
modules/ecs/ipfs/main.tf Outdated Show resolved Hide resolved
modules/ecs/ipfs/versions.tf Outdated Show resolved Hide resolved
modules/ecs/versions.tf Outdated Show resolved Hide resolved
@v-stickykeys
Copy link
Contributor

v-stickykeys commented Apr 8, 2022

I was trying to keep the code in sync with our main TF repo. I can currently run terraform apply from within the app-elp folder in the main repo, or from within the examples/ecs folder in this repo using the same tfvars.

@smrz2001 I don't think this should be the goal. In our 3box labs terraform repo we should be able to use tfvars file in whatever way is easiest for us, but in the examples here we should make it as clear as possible to people how to use the module, ideally with as little effort as possible on their part.

Copy link
Contributor

@v-stickykeys v-stickykeys left a comment

Choose a reason for hiding this comment

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

Just minor things but should be good after that

modules/ecs/ipfs/locals.tf Show resolved Hide resolved
modules/ecs/ipfs/variables.tf Outdated Show resolved Hide resolved
modules/ecs/locals.tf Outdated Show resolved Hide resolved
@smrz2001
Copy link
Collaborator Author

smrz2001 commented Apr 8, 2022

I was trying to keep the code in sync with our main TF repo. I can currently run terraform apply from within the app-elp folder in the main repo, or from within the examples/ecs folder in this repo using the same tfvars.

@smrz2001 I don't think this should be the goal. In our 3box labs terraform repo we should be able to use tfvars file in whatever way is easiest for us, but in the examples here we should make it as clear as possible to people how to use the module, ideally with as little effort as possible on their part.

Makes sense, @v-stickykeys.

We should work towards adding more resources here as well (e.g. S3 bucket, etc.) that must be created outside of these scripts.

@v-stickykeys
Copy link
Contributor

@smrz2001 can you update readme to include instructions about peer id and private key?

…raform-aws

# Conflicts:
#	modules/ecs/ipfs/variables.tf
#	modules/ecs/main.tf
@smrz2001 smrz2001 merged commit 3db99cd into main Apr 9, 2022
@smrz2001 smrz2001 deleted the feature/plat-247-update-terraform-module-terraform-aws branch April 9, 2022 00:19
@smrz2001 smrz2001 restored the feature/plat-247-update-terraform-module-terraform-aws branch April 9, 2022 00:19
@smrz2001 smrz2001 deleted the feature/plat-247-update-terraform-module-terraform-aws branch April 9, 2022 00:19
@smrz2001 smrz2001 restored the feature/plat-247-update-terraform-module-terraform-aws branch April 9, 2022 00:19
@smrz2001 smrz2001 deleted the feature/plat-247-update-terraform-module-terraform-aws branch April 9, 2022 00:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants