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

timetrace delete commands: Default to no when asking for confirmation #147

Merged
merged 1 commit into from
Jul 2, 2021

Conversation

jwnpoh
Copy link
Contributor

@jwnpoh jwnpoh commented Jun 29, 2021

This PR addresses #110.

  • askForConfirmation() now defaults to no unless the input is specifically "y" or "Y".
  • added askForConfirmation() to deleteProjectCommand

Copy link
Collaborator

@aligator aligator left a comment

Choose a reason for hiding this comment

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

@dominikbraun

For me: LGTM

}
}
return true
fmt.Fprint(os.Stderr, "Please confirm [y/N]: ")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually your code looks much cleaner. But I am not sure if @dominikbraun intends to remove the for loop.

I think the for loop is not necessary. So for me it looks ok :-)

Copy link
Owner

Choose a reason for hiding this comment

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

You can throw it out if you want to.

}
}
return true
fmt.Fprint(os.Stderr, "Please confirm [y/N]: ")
Copy link
Owner

Choose a reason for hiding this comment

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

You can throw it out if you want to.

@dominikbraun dominikbraun modified the milestones: timetrace v0.11.0, timetrace v0.12.0 Jul 2, 2021
@dominikbraun
Copy link
Owner

Thanks for solving this, @jwnpoh! Your change will be included in the next release.

@dominikbraun dominikbraun merged commit 85061d5 into dominikbraun:main Jul 2, 2021
@dominikbraun dominikbraun mentioned this pull request Jul 2, 2021
@dominikbraun dominikbraun modified the milestones: timetrace v0.12.0, timetrace v0.11.x Jul 2, 2021
@jwnpoh
Copy link
Contributor Author

jwnpoh commented Jul 2, 2021

Thrilled to have my first merged PR! Thanks for the opportunity!

@jwnpoh jwnpoh mentioned this pull request Jul 4, 2021
@dominikbraun
Copy link
Owner

This PR is included in timetrace v0.11.1.

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.

3 participants