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

Fix unwanted restarting #20295

Merged
merged 1 commit into from
Jul 20, 2018
Merged

Fix unwanted restarting #20295

merged 1 commit into from
Jul 20, 2018

Conversation

volzhs
Copy link
Contributor

@volzhs volzhs commented Jul 20, 2018

Fix #20294

@volzhs volzhs added this to the 3.1 milestone Jul 20, 2018
@volzhs
Copy link
Contributor Author

volzhs commented Jul 20, 2018

@akien-mga @mhilbrunner it's pretty annoying and I think this PR doesn't break anything, so I'll merge it.

@volzhs volzhs merged commit b2d7641 into godotengine:master Jul 20, 2018
@aaronfranke
Copy link
Member

I think this is probably a good change but in the future I'd recommend leaving the PR up for comments for awhile before merging it. Perhaps a day.

@volzhs
Copy link
Contributor Author

volzhs commented Jul 20, 2018

yeah. 99% of my PR was left to be merged by @akien-mga or @mhilbrunner or others.

@volzhs volzhs deleted the fix-restart branch July 20, 2018 02:25
@mhilbrunner
Copy link
Member

I think such decisions are always all nuance and there isn't an objective right way.

Generally, waiting for a day or such is of course better, you're right; but its a tradeoff; the simpler the change and the more annoying the quirk it fixes, the more reasonable it is to merge and ask for forgiveness. ;)

In this case it was fine, I think.

@akien-mga
Copy link
Member

This was fine merging right away, the fix is obviously correct.

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

Successfully merging this pull request may close these issues.

4 participants