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

[deliver] When there is at least 1 framed screenshot, all non-framed screenshot files are skipped, even for devices that have no framed screenshots #15366

Closed
4 tasks done
funnel20 opened this issue Sep 24, 2019 · 12 comments

Comments

@funnel20
Copy link
Contributor

funnel20 commented Sep 24, 2019

New Issue Checklist

Issue Description

We currently only have our iPhone screenshots framed, for iPad not.
When using fastlane deliver it detects some (for iPhone) framed screenshots, and therefor skips all non-framed screenshots. Since we only have non-framed screenshots, no screenshots for iPad are submitted.

Command executed

fastlane deliver

Complete output when running fastlane, including the stack trace and command used
[10:19:30]: Framed screenshots are detected! 🖼 Non-framed screenshot files may be skipped. 🏃
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPad Pro (12.9-inch) (3rd generation) - 01.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPad Pro (12.9-inch) (3rd generation) - 02.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPad Pro (12.9-inch) (3rd generation) - 03.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPad Pro (12.9-inch) (3rd generation) - 04.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPad Pro (12.9-inch) - 01.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPad Pro (12.9-inch) - 02.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPad Pro (12.9-inch) - 03.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPad Pro (12.9-inch) - 04.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone 8 Plus - 01.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone 8 Plus - 02.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone 8 Plus - 03.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone 8 Plus - 05.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone 8 Plus - 06.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone Xs Max - 01.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone Xs Max - 02.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone Xs Max - 03.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone Xs Max - 05.png
[10:19:30]: 🏃 Skipping screenshot file: ./screenshots/en-US/iPhone Xs Max - 06.png 

Environment

🚫 fastlane environment 🚫

Stack

Key Value
OS 10.14.6
Ruby 2.2.4
Bundler? false
Git git version 2.20.1 (Apple Git-117)
Installation Source ~/.rvm/gems/ruby-2.2.4/bin/fastlane
Host Mac OS X 10.14.6 (18G95)
Ruby Lib Dir ~/.rvm/rubies/ruby-2.2.4/lib
OpenSSL Version OpenSSL 1.0.2j 26 Sep 2016
Is contained false
Is homebrew false
Is installed via Fabric.app false
Xcode Path /Applications/Xcode.app/Contents/Developer/
Xcode Version 10.3

System Locale

Variable Value
LANG en_US.UTF-8
LC_ALL en_US.UTF-8
LANGUAGE

fastlane files:

`./Fastfile`
#
#  Fastfile
#  myTifi
#
#  Created by Martijn Koopman on 27/09/17.
#  Copyright © 2017 iMKapps. All rights reserved.
#
#  $Rev: 4769 $
#  $Date:: 2019-04-26 #$

before_all do
    # Set Xcode version
    ENV['DEVELOPER_DIR'] = '/Applications/Xcode 10.1.app/Contents/Developer'
end

# Create a lane for SnapShot:
# Run this via Terminal via: fastlane screenshots
lane :screenshots do
    # Generate screenshots for the App Store:
    snapshot
end

No Appfile found

fastlane gems

Gem Version Update-Status
fastlane 2.130.0 🚫 Update available

Loaded fastlane plugins:

No plugins Loaded

Loaded gems
Gem Version
slack-notifier 2.3.2
rouge 2.0.7
xcpretty 0.3.0
terminal-notifier 2.0.0
multipart-post 2.0.0
word_wrap 1.0.0
public_suffix 2.0.5
babosa 1.0.2
colored 1.2
commander-fastlane 4.4.6
faraday 0.15.4
http-cookie 1.0.3
faraday-cookie_jar 0.0.6
gh_inspector 1.1.3
mini_magick 4.9.5
security 0.1.3
jwt 2.1.0
declarative-option 0.1.0
representable 3.0.4
signet 0.11.0
memoist 0.16.0
googleauth 0.6.7
httpclient 2.8.3
google-api-client 0.23.9
digest-crc 0.4.1
google-cloud-storage 1.16.0
tty-cursor 0.7.0
tty-spinner 0.9.1
tty-screen 0.7.0
json 2.2.0
highline 1.7.10
excon 0.66.0
rubyzip 1.2.4
CFPropertyList 3.0.1
plist 3.5.0
atomos 0.1.3
colored2 3.1.2
nanaimo 0.2.6
xcodeproj 1.12.0
claide 1.0.3
unf 0.1.4
domain_name 0.5.20190701
faraday_middleware 0.13.1
fastimage 2.1.7
bundler 1.16.0
terminal-table 1.8.0
unicode-display_width 1.6.0
addressable 2.7.0
multi_json 1.13.1
os 1.0.1
psych 2.0.8
retriable 3.1.2
mime-types 3.3
mime-types-data 3.2019.0904
uber 0.1.0
declarative 0.0.10

generated on: 2019-09-24

@janpio
Copy link
Member

janpio commented Sep 27, 2019

That is by design. Do you want to have the behavior changed? How should it work then?

@funnel20
Copy link
Contributor Author

The main purpose of fastlane deliver should be to deliver all screenshots from the language folder(s).
That a framed version of a screenshot takes precedence is understandable.
But ignoring all non-framed screenshots seems to fail the main purpose to deliver all screenshots.

What I suggest is that deliver checks for each screenshot whether there is a framed counterpart (can be detected by the suffix _framed in the filename).
In case the framed version exists, this is used, otherwise the non-framed version is used.

Of course it's possible to add a warning or even a question to the user by asking to continue during the initial checks of deliver, in case not all screenshots have a framed counterpart.

@janpio janpio changed the title [Deliver]When there is at least 1 framed screenshot, all non-framed screenshot files are skipped, even for devices that have no framed screenshots [Deliver] When there is at least 1 framed screenshot, all non-framed screenshot files are skipped, even for devices that have no framed screenshots Sep 28, 2019
@janpio janpio changed the title [Deliver] When there is at least 1 framed screenshot, all non-framed screenshot files are skipped, even for devices that have no framed screenshots [deliver] When there is at least 1 framed screenshot, all non-framed screenshot files are skipped, even for devices that have no framed screenshots Sep 28, 2019
@monkeywithacupcake
Copy link

I think I can do a PR for this. Is it okay to work on?

@funnel20
Copy link
Contributor Author

@monkeywithacupcake That would be awesome, yes please.

@fastlane-bot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest fastlane version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

Friendly reminder: contributions are always welcome! Check out CONTRIBUTING.md for more information on how to help with fastlane and feel free to tackle this issue yourself 💪

@fastlane-bot
Copy link

This issue will be auto-closed because there hasn't been any activity for a few months. Feel free to open a new one if you still experience this problem 👍

@monkeywithacupcake
Copy link

This was auto-closed, but I solved the issue with a PR.

@janpio
Copy link
Member

janpio commented Dec 22, 2019

#15491

@janpio janpio reopened this Dec 22, 2019
@fastlane-bot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest fastlane version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
Friendly reminder: contributions are always welcome! Check out CONTRIBUTING.md for more information on how to help with fastlane and feel free to tackle this issue yourself 💪

@ainame
Copy link
Contributor

ainame commented Jan 19, 2021

@monkeywithacupcake I'm very sorry but I closed #15491 since it's pretty old and became a conflict against master now.

The logic in the PR looks like a breaking change to me. For example, frameit would name a "_framed" screenshot from non-suffixed filename; e.g. yourapp6.5_1.png vs yourapp6.5_1_framed.png. In that case, non-suffixed screenshots will also be uploaded when users keep them for some reasons, which is a breaking change.

I think the logic to solve this issue should be like below in order to not affect existing projects using frameit.

  • Check if a screenshot set (a group of screenshots by device type) within a language folder include _framed screenshots
    • YES -> Skip all the screenshots not suffixing with _framed
    • NO -> Upload all the screenshots being the counterpart of the screenshot-set

This way we won't change the semantic that _framed screenshots precedence over non _framed ones and _framed ones aren't mixed up with non _framed ones but it will be more granularly checked. And it meets your desired behaviour to upload screenshots for _framed iPhone screenshots and non _framed iPad screenshots within the same language folder.


Since I heavily refactored deliver's validating logic, I will work on this at some point. (Currently, I'm focusing on #17931 so am not sure when 🙇 )

@fastlane-bot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest fastlane version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

Friendly reminder: contributions are always welcome! Check out CONTRIBUTING.md for more information on how to help with fastlane and feel free to tackle this issue yourself 💪

@fastlane-bot
Copy link

This issue will be auto-closed because there hasn't been any activity for a few months. Feel free to open a new one if you still experience this problem 👍

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

Successfully merging a pull request may close this issue.

6 participants