Skip to content

source: Have ResolveMode implement fmt.Stringer interface#577

Merged
AkihiroSuda merged 1 commit intomoby:masterfrom
tiborvass:source-resolvemode-stringer
Aug 23, 2018
Merged

source: Have ResolveMode implement fmt.Stringer interface#577
AkihiroSuda merged 1 commit intomoby:masterfrom
tiborvass:source-resolvemode-stringer

Conversation

@tiborvass
Copy link
Copy Markdown
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 [email protected]

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 <[email protected]>
@tiborvass tiborvass requested review from AkihiroSuda and ijc August 17, 2018 14:36
@tiborvass
Copy link
Copy Markdown
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
Copy Markdown

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