[resolution pt.2] required package variable source #96
Conversation
perdasilva
force-pushed
the
resolution_pt2_required_package
branch
from
9872bb4
to
d9b2429
Compare
perdasilva
changed the title
perdasilva
changed the title
|
These all seem to build upon each other. So, part 2, includes part 1? And part 3 includes parts 1 and 2? |
Yes, I've been reviewing them individually by looking at changes from only the last commit e.g. for this one: d9b2429 |
Because the merges aren't fast-forward, this might get strange... |
| } | ||
| } | ||
|
|
||
| var _ input.VariableSource = &RequiredPackageVariableSource{} |
There was a problem hiding this comment.
Not sure I get this construct?
There was a problem hiding this comment.
It's ensuring that RequiredPackageVariableSource correctly implements input.VariableSource: https://go.dev/doc/effective_go#blank_implements
| packageName string | ||
| } | ||
|
|
||
| func NewRequiredPackage(packageName string) *RequiredPackageVariableSource { |
There was a problem hiding this comment.
If RequiredPackageVariableSource is exported, why do I need this function?
There was a problem hiding this comment.
Just being more idiomatic, I suppose. Plus if something changes in the initialization we don't need to refactor every call.
| requiredPackage := &RequiredPackageVariableSource{ | ||
| packageName: packageName, | ||
| } | ||
| return requiredPackage |
There was a problem hiding this comment.
Non-blocking nit:
| requiredPackage := &RequiredPackageVariableSource{ | |
| packageName: packageName, | |
| } | |
| return requiredPackage | |
| return &RequiredPackageVariableSource{ | |
| packageName: packageName, | |
| } |
| })) | ||
| }) | ||
|
|
||
| It("should return error if package not found", func() { |
There was a problem hiding this comment.
Non-blocking nit:
| It("should return error if package not found", func() { | |
| It("should return an error if package not found", func() { |
Signed-off-by: perdasilva <perdasilva@redhat.com>
perdasilva
force-pushed
the
resolution_pt2_required_package
branch
from
d9b2429
to
eca26f7
Compare
Signed-off-by: perdasilva <perdasilva@redhat.com>
perdasilva
force-pushed
the
resolution_pt2_required_package
branch
from
eca26f7
to
b6bc92c
Compare
This PR adds the
RequiredPackagevariable source for generating required package variables from package names