Check that team has certificates #81
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
godrei
reviewed
May 29, 2018
codesigndoc/codesigngroup.go
Outdated
| teams := []string{} | ||
| for team := range certificatesByTeam { | ||
| teams = append(teams, team) | ||
| if teamHasCertificates(archiveCertificate, certificatesByTeam[team]) { |
Contributor
There was a problem hiding this comment.
the initial logic here is not correct, this function should work like:
- Do you wasnt to use the archive team for archive? (if multiple team's identity could create this archive)
no-> list the teams that could create this archive and let select - Ipa export method?
- Do you want to use the archive team for the ipa export (if multiple team's identity could export the archive)
no-> list the teams that could export this archive and let select
please update the whole function
godrei
requested changes
May 29, 2018
Contributor
godrei
left a comment
godrei
left a comment
There was a problem hiding this comment.
pls have a look at my comments
…_codesigndoc_distribution_certificate_option_listed
godrei
reviewed
Jun 8, 2018
codesigndoc/codesigngroup.go
Outdated
| _, contains := certificatesByTeam[fmt.Sprintf("%s - %s", archiveCertificate.TeamID, archiveCertificate.TeamName)] | ||
|
|
||
| // Skip if only 1 team has certificates on the machine. | ||
| if len(certificatesByTeam) > 1 && contains { |
Contributor
There was a problem hiding this comment.
warning if the archiving team does not have a certificate for the selected method.
godrei
reviewed
Jun 8, 2018
| if err != nil { | ||
| return certificateutil.CertificateInfoModel{}, fmt.Errorf("failed to read input: %s", err) | ||
| } | ||
| } else if !contains { |
Contributor
There was a problem hiding this comment.
add some comments for this cases, it is a little bit hard to understand
godrei
reviewed
Jun 8, 2018
codesigndoc/codesigngroup.go
Outdated
| // Find the specific development certificate. | ||
| if !exportDistCert { | ||
| certificates := certificatesByTeam[selectedTeam] | ||
| developmentCertificates := certificateutil.FilterCertificateInfoModelsByFilterFunc(certificates, func(certInfo certificateutil.CertificateInfoModel) bool { |
Contributor
There was a problem hiding this comment.
use the certsForSelectedExport
godrei
reviewed
Jun 8, 2018
codesigndoc/codesigngroup.go
Outdated
| // Skip if only 1 team has certificates on the machine. | ||
| if len(certificatesByTeam) > 1 && contains { | ||
| question := fmt.Sprintf(`The archive used codesigning files of team: %s - %s | ||
| Would you like to use this team to sign your project?`, archiveCertificate.TeamID, archiveCertificate.TeamName) |
Contributor
There was a problem hiding this comment.
Would you like to use this team for the .ipa export?
BirmacherAkos
force-pushed
the
no_codesigndoc_distribution_certificate_option_listed
branch
from
4c6454f to
8e25678
Compare
godrei
reviewed
Jun 8, 2018
codesigndoc/codesigngroup.go
Outdated
| // teamHasCertificates returns true if the team has certificate for the selected cert type. | ||
| // If isDistCert == true it will search for Distribution Certificates. If it's == false it will search for Developmenttion Certificates. | ||
| // If the team doesn't have any certificate for the selected cert type, it will return false. | ||
| func teamHasCertificates(isDistCert bool, certificates []certificateutil.CertificateInfoModel) bool { |
Contributor
There was a problem hiding this comment.
this function does not care about the team, rename this function
godrei
reviewed
Jun 8, 2018
codesigndoc/codesigngroup.go
Outdated
| archiveTeam := fmt.Sprintf("%s - %s", archiveCertificate.TeamName, archiveCertificate.TeamID) | ||
|
|
||
| fmt.Println() | ||
| log.Warnf("🚨 Only the archiving team (%s) has certificate for the %s export method", archiveTeam, selectedExportMethod) |
BirmacherAkos
force-pushed
the
no_codesigndoc_distribution_certificate_option_listed
branch
from
349d7b7 to
3d5eb5f
Compare
godrei
reviewed
Jun 11, 2018
codesigndoc/codesigngroup.go
Outdated
| // Skip the question + set the useArchiveTeam = false, if multiple team has certificate for the export method but the archiving team is not one of them. | ||
| if len(filteredCertificatesByTeam) > 1 && contains { | ||
| question := fmt.Sprintf(`The archive used codesigning files of team: %s - %s | ||
| Would you like to use this team to sign your project?`, archiveCertificate.TeamID, archiveCertificate.TeamName) |
Contributor
There was a problem hiding this comment.
Would you like to use this team to export an ipa file?
godrei
reviewed
Jun 11, 2018
codesigndoc/codesigngroup.go
Outdated
| } | ||
|
|
||
| fmt.Println() | ||
| selectedTeam, err = goinp.SelectFromStringsWithDefault("Select the Development team to sign your project", 1, teams) |
Contributor
There was a problem hiding this comment.
please use export an ipa file or similar instead of sign your project
godrei
approved these changes
Jun 11, 2018
BirmacherAkos
deleted the
no_codesigndoc_distribution_certificate_option_listed
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for:
https://discuss.bitrise.io/t/no-codesigndoc-distribution-certificatea/4355