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

Add player: jmitchell #11

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@jmitchell
Copy link
Collaborator

jmitchell commented Jan 5, 2019

No description provided.

@jmitchell jmitchell referenced this pull request Jan 5, 2019

Open

My thoughts and suggestions #17

0 of 6 tasks complete
@jeffkaufman

This comment has been minimized.

Copy link
Owner

jeffkaufman commented Jan 5, 2019

I'm ok with this going in, as long as you promise to submit a PR to remove yourself from players.txt if you get bored and want to stop playing.

(This is only needed until we have a fix for #19)

@dchudz

dchudz approved these changes Jan 5, 2019

@jeffkaufman

This comment has been minimized.

Copy link
Owner

jeffkaufman commented Jan 5, 2019

@dchudz @TheJhyde needs re-approval after merging from master to fix a merge conflict

@jeffkaufman jeffkaufman referenced this pull request Jan 5, 2019

Closed

Update players.txt #12

@pavellishin

This comment has been minimized.

Copy link
Collaborator

pavellishin commented Jan 5, 2019

needs re-approval

This would have been a fun exploit if it didn't require most-recent approvals :P

@jmitchell

This comment has been minimized.

Copy link
Collaborator

jmitchell commented Jan 6, 2019

I tried re-running the travis build about 30m ago and it seems to be stuck in an empty queue.

travis-rerun

It's unclear whether I need to wait longer, or Travis will never actually pick up the re-run request. I'm leaning toward the latter case right now because Travis CI status claims "All Systems Operational" and the "Backlog Linux Builds for Open Source projects" has been at 0 for ~24+ hours.

@jmitchell jmitchell referenced this pull request Jan 6, 2019

Closed

Relax rules for quitting #25

@jeffkaufman

This comment has been minimized.

Copy link
Owner

jeffkaufman commented Jan 6, 2019

it seems to be stuck in an empty queue

Seems to have cleared up. We're still short two approvals to merge.

jeffkaufman added a commit that referenced this pull request Jan 6, 2019

players: switch to a directory
Currently, when new players are added that makes merge conflicts, and requires
a lot of re-approvals.  An easy way to avoid this is to use a directory instead
of a file.

Also add everyone who has asked to be added:
 * Closes #11
 * Closes #12
 * Closes #15
 * Closes #20
 * Closes #22

We should probably merge #23 first, so we don't get stuck if we lose someone
(#19).
@jmitchell

This comment has been minimized.

Copy link
Collaborator

jmitchell commented Jan 6, 2019

The re-run I requested for ad567cd is still enqueued according to GitHub ("Queued 3 hours ago") and unacknowledged by Travis. As far as I can see the last build attempt of that revision "ran 6 hours ago". Unless you're seeing clear evidence of a build for that rev within the past ~3h, I expect we'll see this issue repeat.

@jeffkaufman
Copy link
Owner

jeffkaufman left a comment

testing a rejection

@jeffkaufman
Copy link
Owner

jeffkaufman left a comment

re-approving

@jeffkaufman

This comment has been minimized.

Copy link
Owner

jeffkaufman commented Jan 6, 2019

@jmitchell I just clicked "re-run" which seems to have un-stuck it

@TheJhyde TheJhyde self-assigned this Jan 6, 2019

@TheJhyde TheJhyde self-requested a review Jan 6, 2019

@TheJhyde TheJhyde removed their assignment Jan 6, 2019

@TheJhyde TheJhyde added the Add Player label Jan 6, 2019

@jmitchell

This comment has been minimized.

Copy link
Collaborator

jmitchell commented Jan 6, 2019

I hit re-run again after noticing @TheJhyde approved. This time Travis got to work right away. However, it’s still failing, probably in the master branch validation in the temp directory. @TheJhyde’s approval isn’t reflected in the reviewers list in the log. Not sure yet if this is a logic bug or potentially a caching issue.

@jmitchell

This comment has been minimized.

Copy link
Collaborator

jmitchell commented Jan 6, 2019

Checked again and either user error or a browser caching issue caused me to look at the wrong logs.

Reviews:
  TheJhyde: APPROVED
  jeffkaufman: APPROVED
  pavellishin: APPROVED
Approvals: got 2 (TheJhyde jeffkaufman) needed 3

I think this is blocked on getting approval from @dchudz.

@tnelling

This comment has been minimized.

Copy link
Collaborator

tnelling commented Jan 6, 2019

If we merge #26 (which I think is blocked on approval from @jeffkaufman) we'll be closing this PR anyway.

@jmitchell

This comment has been minimized.

Copy link
Collaborator

jmitchell commented Jan 6, 2019

Thanks, for the reminder @tnelling. I'm going to close this PR to minimize noise.

@jmitchell jmitchell closed this Jan 6, 2019

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