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

Replace --auto-accept with --interactive #3743

Closed
wants to merge 10 commits into from

Conversation

elia
Copy link
Member

@elia elia commented Aug 26, 2020

Description

The current solidus install generator will ask for a number of things unless --auto-accept is provided. Since many of the things asked are not simple yes/no questions, and by default Rails doesn't require any interaction when generating a new app, we should move to an opt-in --interactive flag.

The current PR only switches to the new CLI option and cleans up the generator to make it more maintainable by solidus core and usable by end-users.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

elia added 10 commits August 26, 2020 13:51
Auto-accept erroneously assumed acceptance where the install generator
actively asks for content in several places.
Squiggle heredocs were introduced in Ruby 2.3, so we can finally use
them at large.
Add a description and default value, that is overridden when
solidus_auth_devise will be the auth extension.
The whole class is marked as @Private, so it's fine not to deprecate.
@elia elia mentioned this pull request Sep 30, 2020
5 tasks
@waiting-for-dev
Copy link
Contributor

Hey, @elia 👋 Is this PR still valid?

@elia
Copy link
Member Author

elia commented Jun 15, 2022

@waiting-for-dev I guess the fundamental issue wasn't solved in the meantime so I would say yes, we should still streamline the generator and allow it to run both interactively and unattended.

@waiting-for-dev
Copy link
Contributor

Thanks, @elia! If I get it well, it's more of a semantic renaming than a behavior change, right? I'm ok with that; I think it makes sense. When you have a second, could you rebase it from master, please? 🙏 Also, I'm not sure if it can be already marked as ready for review.

@waiting-for-dev
Copy link
Contributor

Hey @elia, I'll close it for now, but if you find the time to keep working on it I'll be more than happy to reopen 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants