Conversation
| "fmt" | ||
| "sort" | ||
|
|
||
| "github.com/bitrise-io/go-utils/log" |
| } | ||
|
|
||
| // String ... | ||
| func (group SelectableCodeSignGroup) String() string { |
There was a problem hiding this comment.
during the profileutil migration I used a ProfilePrinter structure to overcome the logger dependency: https://github.com/bitrise-io/go-xcode/pull/304/changes#diff-6c679a7bfc6ae37c7cc6d4307691a5af501f7270412e1681b83bf2767bd677ceR19
| // CreateEntitlementsSelectableCodeSignGroupFilter ... | ||
| func CreateEntitlementsSelectableCodeSignGroupFilter(logger log.Logger, bundleIDEntitlementsMap map[string]plistutil.PlistData) SelectableCodeSignGroupFilter { | ||
| return func(group *SelectableCodeSignGroup) bool { | ||
| logger.Debugf("Entitlements filter - removes profile if has missing capabilities") |
There was a problem hiding this comment.
Every filter creator needs a logger, just for debug logging what the filter does, what do you think of doing this logging on the caller side?
There was a problem hiding this comment.
Especially since on the caller side we already have a logger and this is an internal package.
| BundleIDProfilesMap map[string][]profileutil.ProvisioningProfileInfoModel | ||
| } | ||
|
|
||
| func containsCertificate(installedCertificates []certificateutil.CertificateInfoModel, certificate certificateutil.CertificateInfoModel) bool { |
There was a problem hiding this comment.
NIT: I suggest using a bit more generic param names, maybe utilising slices.ContainsFunc and moving the function to the end og the file, as this is a private utility func (less important):
func containsCertificate(list []certificateutil.CertificateInfoModel, item certificateutil.CertificateInfoModel) bool {
return slices.ContainsFunc(list, func(cert certificateutil.CertificateInfoModel) bool {
return cert.Serial == item.Serial
})
}
| } | ||
|
|
||
| if len(matchingProfiles) > 0 { | ||
| sort.Sort(ByBundleIDLength(matchingProfiles)) |
There was a problem hiding this comment.
NIT: this slice sort also could be simpler (and without the additional sort helper structure ByBundleIDLength):
slices.SortFunc(matchingProfiles, func(a, b profileutil.ProvisioningProfileInfoModel) int {
return len(b.BundleID) - len(a.BundleID)
})
The slices.SortFunc comparator returns:
- Negative if
ashould come beforeb - Positive if
ashould come afterb - Zero if they're equal
Since we want to sort by descending length (longest first), we return len(b.BundleID) - len(a.BundleID).
| return groups | ||
| } | ||
|
|
||
| filteredGroups := []SelectableCodeSignGroup{} |
There was a problem hiding this comment.
my IDE highlights these: Empty slice declaration using a literal, it should be var filteredGroups []SelectableCodeSignGroup.
There are a few more of these in the package.
| ) | ||
|
|
||
| // CodeSignGroup ... | ||
| type CodeSignGroup interface { |
| } | ||
|
|
||
| // NewIOSGroup ... | ||
| func NewIOSGroup(certificate certificateutil.CertificateInfoModel, bundleIDProfileMap map[string]profileutil.ProvisioningProfileInfoModel) *Ios { |
There was a problem hiding this comment.
NIT: constructor usually goes right after the structure definition
xcode-archive test: https://app.bitrise.io/build/5b1c7b20-754a-4e94-b156-7e8c6b4b7df8