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

[fastlane][lanemanager] Fastlane command suggestion improvement when running using bundler #18515

Merged
merged 2 commits into from Apr 14, 2021
Merged

[fastlane][lanemanager] Fastlane command suggestion improvement when running using bundler #18515

merged 2 commits into from Apr 14, 2021

Conversation

crazymanish
Copy link
Member

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

  • When running Fastlane using bundler i.e. bundle exec fastlane commands provide us the wrong suggestions.
  • These wrong suggestions are a little confusing for newbie Fastlane users or at-least for junior iOS developers who just join the team and run Fastlane first time and don't have much ruby background 😇

Description

  • Added bundle exec prefix if Fastlane is run using bundler
  • Please see attached screenshots

Testing Steps

  • Update Gemfile and run bundle install
  • gem "fastlane", :git => "git@github.com:crazymanish/fastlane.git", :branch => "lane-manager-improvement"
  • please find attached demo fastfile in zip project
    MyApp.zip

Screenshots

Screenshot 2021-04-08 at 09 39 48

Screenshot 2021-04-08 at 09 35 47

@google-cla google-cla bot added the cla: yes label Apr 8, 2021
ainame
ainame approved these changes Apr 10, 2021
View reviewed changes
Copy link
Contributor

@ainame ainame left a comment

Choose a reason for hiding this comment

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

Nice one! LGTM
I have a minor suggestion. Please address if it makes sense to you🙇

fastlane/lib/fastlane/lane_manager.rb Outdated
@@ -107,12 +107,13 @@ def self.choose_lane(ff, platform)

puts(table)

fastlane_command_prefix = Helper.bundler? ? "bundle exec " : ""
Copy link
Contributor

Choose a reason for hiding this comment

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

"#{fastlane_command_prefix}fastlane"

It's not a big deal but I prefer not to see a variable interpolated and a string literal combined, like above, as I might wonder if there is a whitespace between those two. What do you think about composing all the command name here so that my small concern is resolved? 🙂

Suggested change
fastlane_command_prefix = Helper.bundler? ? "bundle exec " : ""
fastlane_command = Helper.bundler? ? "bundle exec fastlane" : "fastlane"

minuscorp
minuscorp reviewed Apr 10, 2021
View reviewed changes
fastlane/lib/fastlane/lane_manager.rb Outdated
i = UI.input("Which number would you like run?")

i = i.to_i - 1
if i >= 0 && available[i]
selection = available[i].last.pretty_name
UI.important("Running lane `#{selection}`. Next time you can do this by directly typing `fastlane #{selection}` 🚀.")
UI.important("Running lane `#{selection}`. Next time you can do this by directly typing `#{fastlane_command_prefix}fastlane #{selection}` 🚀.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Take in mind to change this if taking @ainame's suggestions

fastlane/lib/fastlane/lane_manager.rb Outdated
@@ -123,7 +124,7 @@ def self.choose_lane(ff, platform)

return platform, lane_name # yeah
else
UI.user_error!("Run `fastlane` the next time you need to build, test or release your app 🚀")
UI.user_error!("Run `#{fastlane_command_prefix}fastlane` the next time you need to build, test or release your app 🚀")
Copy link
Collaborator

Choose a reason for hiding this comment

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

And this 🚀

minuscorp
minuscorp approved these changes Apr 11, 2021
View reviewed changes
Copy link
Collaborator

@minuscorp minuscorp left a comment

Choose a reason for hiding this comment

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

LGTM!🚀

ainame
ainame approved these changes Apr 12, 2021
View reviewed changes
Copy link
Contributor

@ainame ainame left a comment

Choose a reason for hiding this comment

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

🚀

@ainame ainame requested a review from joshdholtz April 12, 2021 03:30
joshdholtz
joshdholtz approved these changes Apr 14, 2021
View reviewed changes
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.

Love this! Thanks for improving this 😊 I appreciate all of your contributions 😍

@joshdholtz joshdholtz merged commit c5210cb into fastlane:master Apr 14, 2021
@crazymanish crazymanish deleted the lane-manager-improvement branch April 14, 2021 04:20
@fastlane-bot
Copy link

Hey @crazymanish 👋

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 🚀

@joshdholtz joshdholtz mentioned this pull request Apr 20, 2021
Merged
@fastlane-bot
Copy link

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

@WangLongONE WangLongONE mentioned this pull request Apr 28, 2021
Closed
@fastlane fastlane locked and limited conversation to collaborators Jun 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@joshdholtz joshdholtz joshdholtz approved these changes

@ainame ainame ainame approved these changes

@minuscorp minuscorp minuscorp 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
@crazymanish @fastlane-bot @joshdholtz @ainame @minuscorp