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

Print line after server is started #1179 (from Flask) #732

Merged
merged 1 commit into from
May 12, 2015

Conversation

indriesergiu
Copy link

Moved the " * Running on host:port" message after the socket was opened for more message accuracy.
pallets/flask#1179

@untitaker
Copy link
Contributor

I think we could move it into serve_forever for even more (theoretical) accuracy?

I like this patch, but FTR: I don't want to guarantee this behavior to the enduser -- they should still handle the case where the server has already printed that line but isn't ready to serve requests yet.

@indriesergiu
Copy link
Author

Done. Moved the message output right before the HTTPServer.serve_forever(self) call.
Also, I agree, this kind of messages should no be relied upon for development. Polling remains the ultimate guarantee :)

Thanks

On Mon, May 11, 2015 at 2:28 PM Markus Unterwaditzer <
notifications@github.com> wrote:

I think we could move it into serve_forever for even more (theoretical)
accuracy?

I like this patch, but FTR: I don't want to guarantee this behavior to the
enduser -- they should still handle the case where the server has already
printed that line but isn't ready to serve requests yet.


Reply to this email directly or view it on GitHub
#732 (comment).

@untitaker
Copy link
Contributor

Could you use rebase -i to squash your commits into one?

@indriesergiu
Copy link
Author

Sorry about that. This is my first time using this fork & pull model. Does it still need cleanup?

@untitaker
Copy link
Contributor

Please reopen, it was fine!

Moved the " * Running on host:port" message right before the HTTPServer.server_forever() call for more message accuracy.
pallets/flask#1179
@untitaker
Copy link
Contributor

You can use git rebase -i on your git branch to edit your commit history into one single commit. Currently you have three (see Commits-tab).

In your case, rebasing is an alternative to merging origin/master into your branch. You execute git rebase origin/master, which applies your commits on top of origin/master (re-creating them, rewriting history) instead of creating a merge commit. This is a good article about rebasing, I'm sure there are better ones too.

So what you should do is:

git rebase -i master

The -i switch will allow you to do all sorts of things with your commits. It will open your commit history in your editor, and you can replace pick/p with f to meld a commit into the previous one. Do so for all commits except the first one.

@indriesergiu indriesergiu reopened this May 11, 2015
@untitaker
Copy link
Contributor

Oh and because you wrote your history, you need to do push -f instead of push. All of this is common practice after somebody is done reviewing a PR, and it's time to clean up the history.

@indriesergiu
Copy link
Author

That was the part I was missing I didn't use force so I saw the previous commits as well. Anyway, I hard reset to the last commit before me, push --force, reapplied patch and push. Hope it's fine now :)

@untitaker
Copy link
Contributor

Thanks, that's fine. I'll wait for Travis to pass until I merge.

@indriesergiu
Copy link
Author

I think the build passed :)

@untitaker
Copy link
Contributor

Great, thanks!

untitaker added a commit that referenced this pull request May 12, 2015
Print line after server is started #1179 (from Flask)
@untitaker untitaker merged commit 00492cd into pallets:master May 12, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants