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

Added warning message to the lockfile waiting routine. #958

Merged
merged 2 commits into from Aug 7, 2017

Conversation

Projects
None yet
3 participants
@matjam
Copy link
Contributor

matjam commented Aug 5, 2017

waiting for something, we at least print a message to stderr about it.

Hopefully will make situations like what is described in #947 obvious.

What does this do / why do we need it?

If dep has to wait for the sm.lock file, currently there's no feedback that its waiting. This provides feedback.

What should your reviewer look out for in this PR?

Do you need help or clarification on anything?

Which issue(s) does this PR fix?

may address #947

Added warning message to the lockfile waiting routine so that if we are
waiting for something, we at least print a message to stderr about it.

Hopefully will make situations like what is described in #947 obvious.

@matjam matjam requested a review from sdboyer as a code owner Aug 5, 2017

@googlebot googlebot added the cla: yes label Aug 5, 2017

@sdboyer sdboyer added the area: gps label Aug 6, 2017

// TODO: #534 needs to be implemented to provide a better way to log warnings,
// but until then we will just use stderr.
//
// #947 appears to be caused by some locking issue; as this is the most likely

This comment has been minimized.

@sdboyer

sdboyer Aug 6, 2017

Member

the note about #534 is good, but let's drop this one, as it either provides the necessary feedback for #947 on its own (and becomes out of date cruft) or isn't actually the problem (and becomes incorrect cruft).

nowtime := time.Now()
duration := nowtime.Sub(lasttime)

if duration > 15*time.Second {

This comment has been minimized.

@sdboyer

sdboyer Aug 6, 2017

Member

seems like this will only print after each 15 second window, rather than printing once initially and then again once every 15 seconds, no?

This comment has been minimized.

@matjam

matjam Aug 6, 2017

Contributor

I'll update the PR, as you're right.

Nathan Ollerenshaw
Removed spurious comment.
Added spurious comment.
Modified logic to display warning initially, and then every 15 seconds, when lockfile busy.
@matjam

This comment has been minimized.

Copy link
Contributor

matjam commented Aug 6, 2017

and we close the PR!

@matjam matjam closed this Aug 6, 2017

@matjam

This comment has been minimized.

Copy link
Contributor

matjam commented Aug 6, 2017

And then open it again ...

@matjam matjam reopened this Aug 6, 2017

@sdboyer

sdboyer approved these changes Aug 7, 2017

Copy link
Member

sdboyer left a comment

cool 👍

@sdboyer sdboyer merged commit 11758a7 into golang:master Aug 7, 2017

3 of 4 checks passed

codeclimate 1 issue to fix
Details
cla/google All necessary CLAs are signed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented Aug 7, 2017

thanks! 🎉

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