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

Adding interactive setup menu and updating readme #300

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xtreme-steve-elliott
Copy link

If you run ./setup.sh without any arguments, it will prompt you as to whether you want to install any opt-ins, and if you respond affirmatively, then it will present you with the list of opt-ins, as read from the file names in the opt-in directory. You can provide it the numbers of the items you want to include, and it will install those.

@joemoore
Copy link
Contributor

joemoore commented Nov 9, 2021

This is a very interesting pull request and seems to address the following concerns:

  • People don't know about opt-ins, what they can choose, or how to choose them.
  • People don't know that analytics are being capture or that they can opt-out of them.

These might be real concerns. It is true that telling people to look in the opt-ins dir is kinda cheesy.

That said, I am inclined to reject this PR and I want to explain why in detail.

Question: is there a way to address these concerns already?
Yes, we can provide better documentation or add simpler features.

Question: what are the rewards vs. risks tradeoffs?
The reward: a more pleasant and informative experience for some.
The risks:

  • setup.sh is now 300% more lines of code, all with untested bash.
  • Considerable maintenance overhead increase with new conditional logic.
  • Bugs?
  • Tempting target for even more tweaking and adding of features: how about arrow-key navigation? Highlight?
  • People might like the just-do-it version we have now without extra prompts.

In summary, I feel the features added in this PR are too fancy and logic-filled for the keep-it-simple ethos of this tool. Colors, numbers, formatting, prompts, parsing -- it's just too much.

Alternatives and Suggestions

  • Improve documentation
  • Consider adding something like ./setup.sh --list or similar to list the opt-ins using the most simple logic possible.

I welcome your thoughts.

@joemoore
Copy link
Contributor

Checking for input again.

@xtreme-steve-elliott
Copy link
Author

I mean, sure, I could add ./setup.sh --list, though I would probably just close this PR and make an entirely different branch at that point, since the majority of the code isn't the part that actually retrieves the list of available opt-ins, and it wouldn't be interactive after scoping it down to that.

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