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

Only ask for usage permission if TTY #697

Merged
merged 2 commits into from
Mar 6, 2019

Conversation

zuzak
Copy link
Contributor

@zuzak zuzak commented Feb 26, 2019

You can run the prototype toolkit in a way that does not accept input.
A basic example would be:

$ npm start | cat

If this is the first time you have run the prototype kit, the
application will hang as there is no way to respond to the question
about recording usage data. This means it is difficult to run the
prototype kit solely without user input.

This commit changes the behaviour of that prompt to only appear if the
user is running the command in a TTY context.

If the user is not running the prototype kit in a TTY, the prompt will
be skipped and the default action will occur as if the user had simply
hit return. In this case, it'll default to "no" and opt the user out of
usage data collection automatically.

A downside of this approach is that the user will not get asked again
if they subsequently run the kit interactively. However, it is unlikely
that users will wish to run the prototype kit initially within a
pipeline whilst also wishing to run it interactively at a later date.


Closes #695.

Another prompt occurs when the chosen port is in use, but
this PR doesn't attempt to edit that (the other prompt is only going to show when things go wrong, whereas this prompt occurs on the initial run of the app, every time).

I haven't included tests as there's no existing tests for the usage data feature.

@kellylee-gds kellylee-gds added Submitted by user issues on behalf of users new 🕔 Hours A well understood issue which we expect to take less than a day to resolve. and removed new labels Feb 27, 2019
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-697 March 4, 2019 16:22 Inactive
Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

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

This seems like a sensible change to me.

Will require a second review from someone else on the team.

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Thanks, @zuzak!

@36degrees
Copy link
Contributor

36degrees commented Mar 5, 2019

Sorry, actually there's one more thing – would you mind rebasing and adding an entry to the changelog?

(We can do this if you prefer.)

@zuzak
Copy link
Contributor Author

zuzak commented Mar 6, 2019 via email

You can run the prototype toolkit in a way that does not accept input.
A basic example would be:

   $ npm start | cat

If this is the first time you have run the prototype kit, the
application will hang as there is no way to respond to the question
about recording usage data. This means it is difficult to run the
prototype kit solely without user input.

This commit changes the behaviour of that prompt to only appear if the
user is running the command in a TTY context.

If the user is not running the prototype kit in a TTY, the prompt will
be skipped and the default action will occur as if the user had simply
hit return. In this case, it'll default to "no" and opt the user out of
usage data collection automatically.

A downside of this approach is that the user will not get asked again
if they subsequently run the kit interactively. However, it is unlikely
that users will wish to run the prototype kit initially within a
pipeline whilst also wishing to run it interactively at a later date.

Closes alphagov#695. Another prompt occurs when the chosen port is in use, but
this commit doesn't attempt to edit that.
@NickColley NickColley merged commit b4709c9 into alphagov:master Mar 6, 2019
@NickColley NickColley mentioned this pull request Mar 6, 2019
@zuzak zuzak deleted the auto-optout-on-noninteractive branch March 11, 2019 10:40
aliuk2012 added a commit that referenced this pull request Apr 2, 2019
Features:
- [#713 Bump GOV.UK Frontend to v2.9.0](#713).

Fixes:
- [#697 Only ask for usage permission if TTY](#697). Thanks [zuzak](https://github.com/zuzak) for this contribution.
- [#712 Turn off npm default auditing](#712).
@aliuk2012 aliuk2012 mentioned this pull request Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕔 Hours A well understood issue which we expect to take less than a day to resolve. Submitted by user issues on behalf of users
Projects
Development

Successfully merging this pull request may close these issues.

5 participants