[WIP] Extend PR statuses to indicate inactivity

[iKevinY]

This PR causes Homu to imitate Bors' behaviour so that "empty statuses" are replaced with "discussing" or "stale", depending on whether the PR can be merged. I guess this is sort of related to #45, though this implementation is of course not based on activity.

Also, the "Number" table header is wider than the 5-digit pull request numbers and that felt like a waste of precious horizontal pixels, so I changed it to "PR #". I can definitely rebase if this change is undesired.

[Gankra]

Discussing was actually only set in bors if someone had actually commented on the PR. Although that's kind of useless because it could be the PR author or someone else just saying r?.

[iKevinY]

Ah. I figured if most of the rows were going to have some sort of status and colour, then "discussing" would serve as a good neutral status (in order to create a spectrum of sorts while scrolling the page).

[barosl]

I'm slightly against this change.

The Status field was originally meant to be mapped directly to the GitHub Statuses API. It has only four statuses: success, failure, error, and pending. That's why Homu didn't have the approved status at first. (Which was changed by #28, though.) As Homu is primarily meant to be used with GitHub, I think this is reasonable.

In bors, there were whole a lot more statuses: bad, stale, discussing, unreviewed, approved, pending, tested, closed. Although they are useful to categorize PRs, I find it too confusing to have so many statuses. Those information should be provided by additional fields IMO, and that's why Homu has dedicated mergeable and assignee fields. This approach will also help us filter PRs by some random condition, as seen by @gankro's #33. If we are to add more "useful-looking" statuses, we would also consider adding some more statuses like unassigned, assigned, and so on.

Also, I think it is good to use the similar terms with other CI solutions. For example, bad in bors was actually error + failure in Homu (and GitHub). tested was success, and as seen above, there are some additional names that are not found in others. When I first saw the bors queue, I was overwhelmed by the number of the statuses it has. I think complying with the de facto standard of the continuous integration world is a good thing to have.

But these are just my preferences, so I want to hear more opinions from others on this issue. I do agree colorizing the queue may increase the readability. So let's see.

[iKevinY]

That's definitely reasonable; I was unaware of the original design decision behind the status field.

Perhaps Homu could be based around four base colours (green, yellow, orange, and red) to represent the four statuses defined in the GitHub API, and then further subcategorize these "base colours" with different shades, ie. approved > approved (for rollup) > success (successful Travis make tidy) > inactive/stale in terms of green saturation. This way, the status column could provide a little more context at a glance while adhering to GitHub's status mappings.

I'm going to work on this a bit to see if I can get an inactivity indicator working as outlined in #45.

[iKevinY]

@barosl This might be an extremely naïve question, but is there some way I can test Homu locally?

[Gankra]

@iKevinY not naive. It's not obvious what's required. iirc you just need to set up the cfg.toml with an API token from github (doesn't need basically any priviledges since it's read-only on a public repo) and you can get an instance going. This is mine:

[main]

token = "PUT_YOUR_GITHUB_API_TOKEN_HERE"
port = 7942
hmac_key = ""
buildbot_key = ""
oauth_client_id = ""
oauth_client_secret = ""

[[repo]]

owner = "rust-lang"
repo = "rust"

reviewers = ["barosl"]
buildbot_url = ""
builders = ["auto-linux", "auto-mac"]

master_branch = "master"
buildbot_branch = "auto"
tmp_branch = "tmp"
rollup_branch = "rollup"

Then you just follow the instructions in the README.

Since it's a stateful app, it won't have all the metadata that the "offical" rust homu has, but it'll get you something.

[iKevinY]

@gankro Thanks! I set up my cfg.toml and everything seems to work great. Turns out I was trying to run Homu using Python 2, which was also problematic.

[barosl]

@iKevinY In addition to the @gankro's comment, I'm also using a local Buildbot instance to debug state changes. It won't be needed just for the presentation layer fixes, so you don't have to set up your own. But if it is required, please tell me. I'll give you permission for my test repository. Or you can just install Buildbot by yourself, which is also not that hard.

[iKevinY]

@barosl Yup, it doesn't look like I'll need to debug any state changes (for this PR at least). Looking to get this done by tomorrow.

[iKevinY]

Currently thinking about something like this (colours chosen completely arbitrarily for now), where "open" signifies non-approved PRs that have been updated in the past week, and "inactive" for those that haven't. This way, they all feel like subsets of one singular GitHub status while providing a little more context. Thoughts?

[iKevinY]

Alright, I think I've resolved all of the merge conflicts. This seems to work on my local machine, but I'm not sure if the inactive statuses will update after the initial pull request retrieval.

[iKevinY]

@barosl Do you have any suggestions on how to have Homu update the inactivity statuses of PRs based on GitHub's webhooks? Would it just be matter of modifying server.github() so that each event_type updates its corresponding state object appropriately?

[barosl]

@iKevinY I think that would work, although there might be some need to limit the event_types that will change the inactivity.

Also, sorry for being late, but I've had some mixed feelings about this PR. Here are my concerns:

1. bors originally listed the PRs in the creation order. I think this was meant to "prioritize" the older PRs, as in the sense of first-come, first-served. And, Homu inherited this behavior. Your PR distinguishes the "inactive" ones from the "active" ones, which basically changes this philosophy.

   I'm not entirely opposed to your approach. In fact, I think it has the possibility to improve the overall performance. However, I'm also not sure it doesn't harm the usefulness of the chronological order.

2. Colors. Your PR uses the shades of green for the "open" and "inactive" states. However, I would like to see a clear distinction between them and the "approved" state, which also uses green.

[iKevinY]

@barosl The "first-come, first-served" philosophy definitely makes sense. I didn't mean to disrupt the original order scheme; the fact that that these changes would prevent inactive PRs from rising to the top completely slipped my mind.

With regards to the colours, what do you propose? For this proof-of-concept, I stuck to using shades of green to try to match the GitHub API status idea that you mentioned in an earlier comment.

That being said, if you feel that this PR is straying too far away from your idea of how Homu should look, I would be happy to change the scope of the changes (ie. by adding an additional "last modified" column instead of representing this information in the status column, for instance).

[iKevinY]

Going to close this pull request since the scope doesn't really fit Homu for the time being.