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.

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

[screengrab] strip whitespace from device_ext_storage to get rid of newline on Windows #13065

Merged
merged 1 commit into from Aug 14, 2018

Conversation

janpio
Copy link
Member

@janpio janpio commented Aug 8, 2018

On Windows the executed adb command returns the wanted string - and a newline. To make sure this works everywhere, we .strip the string before working with it.

closes #13061

@joshdholtz

This comment has been minimized.

@janpio janpio force-pushed the janpio-fix_screengrab_on_windows_2 branch from 9c93d86 to 83d08bb Compare August 13, 2018 11:09
@janpio

This comment has been minimized.

@@ -136,9 +136,10 @@ def screenshot_file_names_in(output_directory, device_type)
end

def determine_external_screenshots_path(device_serial)
device_ext_storage = run_adb_command("adb -s #{device_serial} shell echo \\$EXTERNAL_STORAGE",
device_ext_storage = run_adb_command("adb -s #{device_serial} shell echo \$EXTERNAL_STORAGE",
Copy link
Member

Choose a reason for hiding this comment

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

Commented this on the other PR that this is based off of but just wanted to put this here to so that this doens't accidentally get merged in but \\ is needed on mac 😊

(This will get addressed once rebased after other PR gets merged in to master)

@janpio janpio added this to the Windows Support milestone Aug 13, 2018
@joshdholtz
Copy link
Member

@janpio Merged your other PR. Did you want to rebase this one now?

@janpio janpio force-pushed the janpio-fix_screengrab_on_windows_2 branch from 83d08bb to ef45acd Compare August 14, 2018 13:29
@janpio
Copy link
Member Author

janpio commented Aug 14, 2018

@joshdholtz Rebased and changed PR description.

Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

Looks good and works good! 👍

@janpio janpio merged commit 0bf2e8c into master Aug 14, 2018
@janpio janpio deleted the janpio-fix_screengrab_on_windows_2 branch August 14, 2018 14:13
@fastlane-bot
Copy link

Hey @janpio 👋

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 🚀

@fastlane-bot
Copy link

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

@fastlane fastlane locked and limited conversation to collaborators Oct 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[screengrab] determine_external_screenshots_path returns /sdcard\n instead of /sdcard
5 participants