-
-
Notifications
You must be signed in to change notification settings - Fork 197
Revisit the manual signing, revert the dialogs and add --provision switch #2393
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
Conversation
cff49e7
to
25a30f8
Compare
25a30f8
to
02e9141
Compare
02e9141
to
59bc650
Compare
@@ -248,6 +258,13 @@ export class IOSProjectService extends projectServiceBaseLib.PlatformProjectServ | |||
basicArgs.push("-xcconfig", path.join(projectRoot, this.$projectData.projectName, "build.xcconfig")); | |||
} | |||
|
|||
// if (this.$logger.getLevel() === "INFO") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this code commented? Do we need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It partially addresses the #2006
I've tested it locally but QAs won't have time to push it for this release and I didn't bother to go the right way and make a separate branch with this change only to merge in separate PR later. I hope we will uncomment this shortly after the release.
if (signing && signing.style === "Manual") { | ||
for(let config in signing.configurations) { | ||
let options = signing.configurations[config]; | ||
if (options.name !== this.$options.provision && options.name !== this.$options.provision) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both statements check the same conditions, maybe we really want to be sure that options.name !== this.$options.provision
😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should have been .uuid
if (this.$options.provision) { | ||
const pbxprojPath = path.join(projectRoot, this.$projectData.projectName + ".xcodeproj", "project.pbxproj"); | ||
const xcode = Xcode.open(pbxprojPath); | ||
const signing = xcode.getSigning(this.$projectData.projectName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can extract the whole code from here to line 358 in a separate method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -58,7 +58,7 @@ | |||
"mute-stream": "0.0.5", | |||
"open": "0.0.5", | |||
"osenv": "0.1.3", | |||
"pbxproj-dom": "1.0.3", | |||
"pbxproj-dom": "^1.0.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not very safe - in case pbxproj-dom
is released with breaking change or some unexpected behavior for us in a minor version, we'll be broken on live. That's why we prefer strict versions in package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@rosen-vladimirov I will adress all the comments in following PRs, the version should probably be fixed ASAP but the rest will probably happen shortly after the release. |
No description provided.