Skip to content

Fix error after creating .dogrc when not answering y or n at the prompt#582

Merged
zippolyte merged 3 commits intoDataDog:masterfrom
NassimBounouas:580-dogrc-created-twice
Jun 30, 2020
Merged

Fix error after creating .dogrc when not answering y or n at the prompt#582
zippolyte merged 3 commits intoDataDog:masterfrom
NassimBounouas:580-dogrc-created-twice

Conversation

@NassimBounouas
Copy link
Copy Markdown
Contributor

Requirements for Contributing to this repository

  • Fill out the template below. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • The pull request must only fix one issue, or add one feature, at the time.
  • The pull request must update the test suite to demonstrate the changed functionality.
  • After you create the pull request, all status checks must be pass before a maintainer reviews your contribution. For more details, please see CONTRIBUTING.

What does this PR do?

This pull request is related to bug presented in the issue #580

Description of the Change

The while loop used to wait the user entry was set to break only if y or n was found but the empty response permits to create the configuration file (response.strip().lower() not in ['', 'y', 'n'])

I set the default response as None and make the while continues if the response is None or different of ['', 'y', 'n'].

Alternate Designs

I thought about using a non empty string as the default response, for example 'None' instead of None to lighten the while condition but is a bit cheap from my point of view.

Verification Process

  1. I reproduced the bug
  2. I made my modification
  3. I reinstalled the package
  4. I tried to reproduce the bug without success

Additional Notes

It's my first PR on datadog, I followed the CONTRIBUTING.md let me know if I missed a step.

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

@NassimBounouas NassimBounouas requested a review from a team as a code owner June 28, 2020 04:03
Copy link
Copy Markdown
Contributor

@zippolyte zippolyte 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 this fix. I added one suggestion, to avoid the same bug when answering yes.

while response is None or response.strip().lower() not in ['', 'y', 'n']:
response = get_input('%s does not exist. Would you like to'
' create it? [Y/n] ' % config_file)
if response.strip().lower() in ['', 'y', 'yes']:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if response.strip().lower() in ['', 'y', 'yes']:
if response.strip().lower() in ['', 'y']:

Copy link
Copy Markdown
Contributor Author

@NassimBounouas NassimBounouas Jun 30, 2020

Choose a reason for hiding this comment

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

Hi @zippolyte,

Indeed, I didn't see this one ! I applied the additional fix !

Edit : I think both commits can be squashed at the merge.

@zippolyte
Copy link
Copy Markdown
Contributor

/azp run DataDog.datadogpy.integration

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@zippolyte zippolyte merged commit 3b5b031 into DataDog:master Jun 30, 2020
@zippolyte zippolyte added changelog/Fixed Fixed features results into a bug fix version bump resource/dogshell labels Jun 30, 2020
@zippolyte zippolyte changed the title Fix Stop while loop after .dogrc creation if only return is pressed Fix error after creating .dogrc when not answering y or n at the prompt Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/Fixed Fixed features results into a bug fix version bump resource/dogshell

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants