Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix logarchive collection on Xcode 8.3 #8848

Merged
merged 3 commits into from
Apr 12, 2017
Merged

Fix logarchive collection on Xcode 8.3 #8848

merged 3 commits into from
Apr 12, 2017

Conversation

endocrimes
Copy link
Collaborator

This changes log archive collection to specify the --output parameter on
simctl spawn {id} log collect to fix an issue where the log archive would be
written to an incorrect path when running undner Xcode 8.3

  • Does this parameter exist on 8.x < 8.3? (if this PR passes, then it works at least on 8.2)

8.3 PR Failing because of this here:

Example project:

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@endocrimes endocrimes force-pushed the dani_device_manager_copy_logarchive branch from 685f545 to 7e7de74 Compare April 11, 2017 14:43
@googlebot
Copy link

CLAs look good, thanks!

@endocrimes endocrimes force-pushed the dani_device_manager_copy_logarchive branch 3 times, most recently from 299633a to ae26e2d Compare April 11, 2017 15:13
@endocrimes endocrimes force-pushed the dani_device_manager_copy_logarchive branch from ae26e2d to 93005eb Compare April 11, 2017 15:22
lyndsey-ferguson
lyndsey-ferguson reviewed Apr 11, 2017
View reviewed changes
fastlane_core/lib/fastlane_core/device_manager.rb

FileUtils.mkdir_p(logarchive_dst)
FileUtils.cp_r("#{logarchive_src}/.", logarchive_dst)
UI.success "Copying file '#{logarchive_src}' to '#{logarchive_dst}'..."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the log message, can we have it back?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyndsey-ferguson We never actually copy from a src to a destination dir now tho? - We could potentially say something like "Exporting logarchive to {path}"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had thought of using the return value from the execute, but looking at this, we are already printing the command, so I don't think this is necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

lyndsey-ferguson
lyndsey-ferguson approved these changes Apr 11, 2017
View reviewed changes
Copy link
Contributor

@lyndsey-ferguson lyndsey-ferguson left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than my ask, looks good (and making sure it works with Xcode 8.2.1)

@endocrimes
Copy link
Collaborator Author

image
Appears to have the same option on 8.2.1 :D

@endocrimes endocrimes mentioned this pull request Apr 11, 2017
Closed
@KrauseFx KrauseFx merged commit 61e5522 into master Apr 12, 2017
@KrauseFx
Copy link
Member

Beautiful, thanks @dantoml 👍

@KrauseFx KrauseFx deleted the dani_device_manager_copy_logarchive branch April 12, 2017 01:47
@fastlane-bot
Copy link

Hey @dantoml 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

@KrauseFx KrauseFx mentioned this pull request Apr 14, 2017
Merged
@fastlane-bot
Copy link

Congratulations! 🎉 This was released as part of fastlane 2.27.0 🚀

@fastlane fastlane locked and limited conversation to collaborators Jul 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lyndsey-ferguson lyndsey-ferguson lyndsey-ferguson approved these changes

@KrauseFx KrauseFx KrauseFx approved these changes

Assignees
No one assigned
Labels
cla: yes status: released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@endocrimes @googlebot @KrauseFx @fastlane-bot @lyndsey-ferguson