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

[snapshot] added check for missing Snapshot Helper File (#13038) #13205

Merged
merged 3 commits into from
Aug 28, 2018
Merged

[snapshot] added check for missing Snapshot Helper File (#13038) #13205

merged 3 commits into from
Aug 28, 2018

Conversation

JonnyBeeGod
Copy link
Contributor

@JonnyBeeGod JonnyBeeGod commented Aug 24, 2018

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

Fixes #13038

Description

Added check for missing Snapshot Helper File. If there is no Snapshot Helper File found, snapshots exits with a descriptive user error.

I tested the fix via manually deleting the Snapshot Helper File from my project and running fastlane snapshot afterwards.

@googlebot

This comment has been minimized.

@JonnyBeeGod

This comment has been minimized.

@googlebot

This comment has been minimized.

@janpio
Copy link
Member

janpio commented Aug 24, 2018

Awesome!

What is missing right now is some instruction on how to do create the file. The best source I could find was https://docs.fastlane.tools/actions/snapshot/#quick-start. Maybe add that (or something better I missed) to the error message?

@JonnyBeeGod
Copy link
Contributor Author

You're right that's more user friendly. Hope the wording is ok

Copy link
Member

@janpio janpio left a comment

Choose a reason for hiding this comment

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

I like it! 🥇

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.

Two small nitpicks just to keep some UI formatting consistent in this file 😊

@@ -122,6 +122,15 @@ def verify_helper_is_current
UI.verbose("Checking that helper files contain #{current_version}")

helper_files = Update.find_helper
if helper_files.empty?
UI.error("Your Snapshot Helper file is missing, please place a copy")
UI.error("in your project directory.")
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove the "." on this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

UI.error("in your project directory.")
UI.message("More information about Snapshot setup can be found here:")
UI.message("https://docs.fastlane.tools/actions/snapshot/#quick-start")
UI.user_error!("Please add a Snapshot Helper file to your project.")
Copy link
Member

Choose a reason for hiding this comment

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

Can we also remove the "." on this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@JonnyBeeGod
Copy link
Contributor Author

Right 😁 Fixed it now

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.

Thanks for making those changes and thanks for the contribution! 💪 ❤️

@joshdholtz joshdholtz merged commit 8f0f410 into fastlane:master Aug 28, 2018
@JonnyBeeGod JonnyBeeGod deleted the Improve_SnapshotHelper branch August 31, 2018 04:12
janpio pushed a commit to janpio/fastlane that referenced this pull request Sep 11, 2018
…) (fastlane#13205)

* [snapshot] added check for missing Snapshot Helper File (fastlane#13038)

* [snapshot] added link to snapshot quick setup in error message (fastlane#13038)

* [snapshot] UI formatting (fastlane#13038)
@fastlane fastlane locked and limited conversation to collaborators Nov 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[snapshot] Improve SnapshotHelper.swift handling if file doesn't exist
4 participants