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

settings: Make root modules configurable #198

Merged
merged 1 commit into from
Jul 10, 2020

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Jul 1, 2020

Depends on #196

Closes #189

Related: hashicorp/vscode-terraform#423

@radeksimko radeksimko added the enhancement New feature or request label Jul 1, 2020
@aeschright aeschright self-requested a review July 1, 2020 19:02
@radeksimko radeksimko force-pushed the f-configurable-root-modules branch 3 times, most recently from 1b15cb6 to b471876 Compare July 3, 2020 09:12
@radeksimko radeksimko marked this pull request as ready for review July 3, 2020 09:13
docs/SETTINGS.md Outdated Show resolved Hide resolved
@radeksimko radeksimko added this to the v0.5.0 milestone Jul 3, 2020
@radeksimko radeksimko force-pushed the f-configurable-root-modules branch from b471876 to ce9337d Compare July 3, 2020 14:32
@radeksimko

This comment has been minimized.

@radeksimko radeksimko marked this pull request as draft July 3, 2020 15:44
@radeksimko radeksimko force-pushed the f-configurable-root-modules branch from ce9337d to 8bd954d Compare July 7, 2020 13:17
@radeksimko

This comment has been minimized.

@radeksimko radeksimko self-assigned this Jul 10, 2020
@radeksimko radeksimko force-pushed the f-configurable-root-modules branch from 8bd954d to 3805145 Compare July 10, 2020 11:26
@radeksimko radeksimko force-pushed the f-configurable-root-modules branch from 3805145 to f183ba7 Compare July 10, 2020 13:58
@radeksimko radeksimko marked this pull request as ready for review July 10, 2020 14:50
@radeksimko radeksimko requested review from a team and removed request for aeschright July 10, 2020 14:59
Copy link
Contributor

@appilon appilon left a comment

Choose a reason for hiding this comment

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

minor nitpick

@@ -198,3 +199,15 @@ func RootModuleWalker(ctx context.Context) (*rootmodule.Walker, error) {
}
return w, nil
}

func WithRootModuleLoader(rml rootmodule.RootModuleLoader, ctx context.Context) context.Context {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, make ctx first argument

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't disagree, but I think I'd prefer to change all of the With* functions at the same time and perhaps do that in a separate PR, if that's ok and follow the same pattern throughout the package for now?

@ghost
Copy link

ghost commented Aug 9, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the context necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Aug 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow user to override discovery of root modules
3 participants