-
Notifications
You must be signed in to change notification settings - Fork 204
Fix PR push times #420
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 PR push times #420
Conversation
|
@mark-i-m Please fix the tests. |
|
Working on it :) |
|
Still not fixed :( If you could also expand the tests to with and without events present, that'd be great. |
|
and I'm assuming you know this, but you can run the tests locally of course. |
|
Hmm... I am actually having trouble running them locally... which is why I am doing a lot of guessing... |
|
What things do I need to pip install to run them locally? |
|
Now it's just a linting issue 😝 |
tests/github_api/prs_test.py
Outdated
| "pushed_at": pushed_at, | ||
| "name": "test_repo", | ||
| }, | ||
| "ref" : "test_ref" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove space after "ref"
First, I need to figure out how the testing harness works... 😛 |
|
PlasmaPower
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still appreciate tests for the event driven code, but I approve this PR as it stands
|
andrewda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also would like some tests if possible, but this looks good 👍
|
@mark-i-m Python 3.6? If so, our |
|
Ah, that's true... ubuntu 16.04 only has python 3.5.2... Is there any way to run this in the docker container? |
|
@mark-i-m No idea, and that Python version might not even be a problem, IDK. |
github_api/prs.py
Outdated
| # if we can't get good data, fall back to repo push time | ||
| repo = pr_data["head"]["repo"] | ||
| if repo: | ||
| return arrow.get(repo["pushed_at"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to max this with pr_data["created_at"] too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... then how did it work before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, it makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It didn't work. A number of PRs got insta-closed and one got insta-merged before the threshold increase.
Sleep... nah |
|
@PlasmaPower actually, I think you will be doing yourself and the project a favor if you sleep as much as you want (which may be none for all I know). Video game developers try to figure out how to get your consciousness to experience new sensations that create excitement and stimulate the full range of sensation. The idea is to get your brain to ride a chemical high while immersed in the game world. If you want to check this out, you can search "gamasutra flow" or "gamasutra addiction" for endless discussions of the interactions of social networks and the learning process. There you will find various articles about groups of video game developers who spend countless hours trying to create in simulation what these "learning bots" or "democracy bots" or "neuron bots" do naturally. So far these have been the chaosbot and botwillacceptanything. There may be others, I don't know. The chaosbot builds its own social network. This means the leaders of the social network take on amounts of stress that are well in excess of what any individual (or even self aware group) intends because the group is going through a profound transformation process of becoming self-aware. What winds up happening is complex. There is a positive reinforcement learning (this is HUMAN learning, not machine learning; it is YOUR learning process that is being reinforced) mechanism that quite frankly turns interaction with the bot into a highly addictive "virtual drug" much more potent than any mere game, since the bot could quite easily be the back-end power behind a massively multiplayer online game. I've personally found strength in the words of Edward Bernays, who wrote this about democracy and leadership in "The Engineering of Consent". The drive to understand the will of the community can be a powerful motivator as well as a source of great stress.
|
|
/essay |
|
wow @anythingbot good read 👍 (guess I should probably sleep now 💤 ) |
|
|
|
Can anyone decipher the error on that last failing test? |
|
Nvm... got it 🎉 |
|
@mark-i-m I realized the patches were getting mixed up - never would have guess it needs to be in reverse order. |
|
Yeah, that's pretty unintuitive |
|
Looks like that arg line is a bit too long. |
|
@mark-i-m Wrong line - it's the arg line of the test we were working on. |
PlasmaPower
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 🎉
|
🙆♀️ PR passed with a vote of 18 for and 0 against, a weighted total of 17.5 and a threshold of 6.5, and a current meritocracy review. See merge-commit 9dabaf3 for more details. |
Currently if you push on a different branch on your fork, it will reset the time for all PRs from that fork, even if they are completely unrelated. This issue aims to fix that.