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

source: Have ResolveMode implement fmt.Stringer interface #577

Merged
merged 1 commit into from
Aug 23, 2018

Conversation

tiborvass
Copy link
Collaborator

Out of the two ResolveMode types in buildkit, only the lower-level one in client/llb
had a String() method. This patch makes the ResolveMode type from the source package
also have a String() method.

Signed-off-by: Tibor Vass tibor@docker.com

Out of the two ResolveMode types in buildkit, only the lower-level one in client/llb
had a String() method. This patch makes the ResolveMode type from the source package
also have a String() method.

Signed-off-by: Tibor Vass <tibor@docker.com>
@tiborvass
Copy link
Collaborator Author

@AkihiroSuda @ijc I'm not 100% sure if I'm doing what's expected or if there's refactoring to be done. My understanding is that these types although they seem duplicates, they are intended to be used at two different level of abstractions. Let me know if I misunderstood, in which case I could refactor, I just don't want to clutter the dependency graph more.

@ijc
Copy link
Collaborator

ijc commented Aug 17, 2018

I'm not sure about the duplication either, but adding this for now seems harmless. I'm also not entirely clear why these are int and not just string (or even aliases to the underlying pb string) to start with.

@AkihiroSuda AkihiroSuda merged commit f71f4b6 into moby:master Aug 23, 2018
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.

3 participants