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

Add support for custom working directory #61

Conversation

ThorstenHans
Copy link
Contributor

@ThorstenHans ThorstenHans commented Apr 9, 2021

With this PR, users can specify a custom working directory for theit Terraform project.

  • Tests
  • Docs (Readme)

Signed-off-by: Thorsten Hans <thorsten.hans@gmail.com>
@ThorstenHans ThorstenHans force-pushed the feature/custom-working-directory branch from 799adcf to 7a8a362 Compare April 9, 2021 13:59
@carolynvs
Copy link
Member

I have removed the comment that this fixes #16. For #16 we should first update the builder library so that it supports changing the workding directory, and then use that in the terraform mixin.

getporter/porter#1403

@vdice
Copy link
Member

vdice commented Apr 14, 2021

I'm unclear whether we want this change even if/when we add changes to fix #16 . The changeset here looks good to me, if we wish to proceed with both.

@carolynvs
Copy link
Member

Now that we have built-in support for setting the working directory on a mixin, here's how a mixin can support this too:

  1. Update the dependency on porter in their go.mod file to v0.38.0+
  2. Add a field "dir" to the mixin step instruction, just like the exec mixin
    https://github.com/getporter/porter/blob/aa542e4cefe02605ffc9667b8af65cbdcccf022d/pkg/exec/action.go#L102
  3. Implement the updated executable step interface
    https://github.com/getporter/porter/blob/aa542e4cefe02605ffc9667b8af65cbdcccf022d/pkg/exec/action.go#L138-L140

@vdice
Copy link
Member

vdice commented May 13, 2021

@ThorstenHans now that it is possible to add support in this mixin for the standardized method of setting the working dir per step, do we still wish to add this functionality at the mixin config level?

@ThorstenHans
Copy link
Contributor Author

Obviously, the mixin should adopt the feature from porter instead of adding support for working directory by itself.

@ThorstenHans
Copy link
Contributor Author

Should I create a dedicated PR (for updating the porter dep) and add an example there or could I reuse this PR instead

@vdice
Copy link
Member

vdice commented May 18, 2021

@ThorstenHans Although I might have a slight preference towards the creation of a new PR, I'll leave it up to you - whichever is most convenient.

@ThorstenHans
Copy link
Contributor Author

@vdice that's fine for me I'll close this one 👍

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.

None yet

3 participants