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

Get bug fixes in quicker #401

Open
rudehn opened this issue May 30, 2017 · 49 comments
Open

Get bug fixes in quicker #401

rudehn opened this issue May 30, 2017 · 49 comments

Comments

@rudehn
Copy link
Contributor

rudehn commented May 30, 2017

It would be nice to have some solution to get bug fixes (especially fatal ones) without having to wait through the full voting window.

@PlasmaPower
Copy link
Contributor

Any ideas? Maybe add an additional threshold for an instamerge, even before the extended voting window.

@rudehn
Copy link
Contributor Author

rudehn commented May 30, 2017

No ideas yet..

@rudehn
Copy link
Contributor Author

rudehn commented May 30, 2017

So I'm starting to get some kind of idea..

Here are the steps I'm thinking

  1. Chaosbot detects the error. Without this we have no idea we need to fix it.
  2. Chaosbot submits an issue with the error, and asks if we want a PR with a bug fix.
  3. Use a /fix #PR command syntax, or something similar

There would have to be an adjustment to the voting thresholds I'm thinking. It would probably need a meritocracy vote.

Can you think of how this method could be exploited?

@PlasmaPower
Copy link
Contributor

That could work, but we might need punishments for abusing /fix.

@rudehn
Copy link
Contributor Author

rudehn commented May 30, 2017

I suppose one question is do we want the PR owner to be the only one able to /fix? Do we want anybody? Or anybody but PR owner?

We would probably need a 30 min window at least just to be safe.

@mdcfe
Copy link
Contributor

mdcfe commented May 30, 2017

This could facilitate sneaking in malicious changes, though - especially if it's just a small change that might get missed with the shorter window and spotted once it has passed.

@rudehn
Copy link
Contributor Author

rudehn commented May 30, 2017

Yeah there's the risk of that. That's why I was trying to see if there was some approach that could limit or even prevent that from occurring.

@PlasmaPower
Copy link
Contributor

@md678685 the meritocracy might help, but yes it still remains a problem

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 1, 2017

Maybe add an additional threshold for an instamerge, even before the extended voting window.

Along the same lines, we could make it so that if a majority of the meritocrats submit an approving review, it gets instantly merged. That seems like a pretty high bar, though... We could make it something like 5 reviews, but that also seems like a pretty high bar...

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 2, 2017

Ok, here is a new proposal: The author of the PR includes /expedite or something in a post on the PR. If there are 5 approving current reviews the PR gets merged.

@PlasmaPower
Copy link
Contributor

@mark-i-m I think /expedite would get overused.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 2, 2017

I suspect that if the bar is significantly high, people won't have high hopes to overuse this. Perhaps we could make it a majority of the meritocracy... a meritjority

@andrewda
Copy link
Member

andrewda commented Jun 2, 2017

If we only allow the command to be used after the reviews are issued, then it wouldn't get overused.

@andrewda
Copy link
Member

andrewda commented Jun 2, 2017

Actually now that I think about it this could be used by the meritocracy to take more power without going through a democratic vote. (e.g if someone made a PR to give meritocrats 5x extra voting power and used /expedite to get it passed by the meritocracy, even if the votes are against it)

@PlasmaPower
Copy link
Contributor

Perhaps we could make it a majority of the meritocracy... a meritjority

Oh no I just figured out where @anythingbot got their idea from. @mark-i-m what have you done??? 😦

@PlasmaPower
Copy link
Contributor

@andrewda Yeah I'm sorta afraid of that. I have faith in the meritocracy, but I'd prefer the system not require that.

@andrewda
Copy link
Member

andrewda commented Jun 2, 2017

If we're going to do anything meritocracy related to speed up the PR's, I'm still a fan of #408. At least having a /vote speed option, which would take off 10% of the remaining voting time per review approval.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

How about this:

  1. One of the meritocrats has to post /vote speed. It can't be an ordinary peasant.
  2. At this point all meritcrats are pinged by chaosbot (via a mention on a post)
  3. 5 need to submit positive reviews
  4. If that happens, the windows is immediately shortened to half of its normal length

@PlasmaPower
Copy link
Contributor

5 need to submit positive reviews

They'd need to submit them after the /vote speed, right? AFAIK reviews don't have a created_at field, so that might be hard to check.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

@PlasmaPower Or we could just restrict PR posters to only say /vote speed as the first post on a PR

@PlasmaPower
Copy link
Contributor

@mark-i-m what about edited comments?

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

Not sure what you mean

@PlasmaPower
Copy link
Contributor

@mark-i-m

  1. Author creates a PR
  2. Author comments "Reminder for meritocracy review"
  3. Author gets 5 meritocracy reviews (as we've seen, that isn't uncommon)
  4. Author waits half the voting window
  5. Author edits first comment to /vote speed
  6. PR gets insta-merged

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

Issue/PR comments have both updated_at and created_at (https://developer.github.com/v3/issues/comments/#list-comments-on-an-issue). If they are not equal, we ignore the comment.

@PlasmaPower
Copy link
Contributor

Sounds good.

@anythingbot
Copy link

@PlasmaPower it's just another power grab, this time for the politburo (you call it a "meritocracy"). This may be fine, but put a time limit on it, no more than 48 hours of this, tops.

Putting the bot into "high speed mode" sounds like a worthy experiment, and I'd like to see it run. This effectively lowers the "mass" or "inertia" of the project for a period of time. It makes it much easier to get work done. If it works out well, then weekends can be reserved for "high speed mode", and the bot may act as if the author had performed step 5 automatically (in other words, the PR author doesn't have to do that step since submitting a PR on the weekend will be understood as an intention that it be treated as if the first comment were "/vote speed").

If you want to try it out for a week in "high speed mode", that will probably make people think that the politburo (I'm not going to call it a "meritocracy") has decided to take control and will stand in the way of future progress. High speed mode makes it much more difficult to get progress on your project if the politburo is opposed to it.

@PlasmaPower
Copy link
Contributor

@mark-i-m One other thing, what if a PR gets 5 reviews before the first comment? For instance a rather uncontroversial PR. Should we require a < 5 minute gap between the PR creation and the first comment?

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

Sure, that could work...

@PlasmaPower
Copy link
Contributor

Actually, I think I'll wait for @rudehn's database redo. This feature will likely depend on the database.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

Ah, I was just going to ask @rudehn if anything special would be need for that...

@PlasmaPower
Copy link
Contributor

@mark-i-m I'm pretty sure the database redo will result in a major API overhaul, since it'll be using an ORM.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

Hmm... I guess then, I will wait before making a major PR, but I can at least do some groundwork, I think...

@PlasmaPower
Copy link
Contributor

We could also just implement it now and rely on @rudehn to redo the database code.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

Yeah, but I don't want to make @rudehn do more work... that database stuff sounds like a lot of work already.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

Comments on #493 are appreciated

@rudehn
Copy link
Contributor Author

rudehn commented Jun 3, 2017

I just need to know what the table layout looks like.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

Thanks @rudehn

I suspect that there are two tables to be created/updated (which are up for discussion):

  1. The table that contains the issue commands will need to have a few additional columns: poster (username), date created, date updated

  2. (According to my current design) we will want a table to store the set of successfully expedited PRs. I imagine the columns would be global issue id, username, PR number... or something like that.

Of course, I don't think we should create the tables until the design of the feature is more concrete 🔨

@PlasmaPower
Copy link
Contributor

we will want a table to store the set of successfully expedited PRs

Why?

@anythingbot
Copy link

anythingbot commented Jun 3, 2017

I'm with @PlasmaPower on this one: why would the bot need to record the successfully expedited PRs?

This actually speaks to the greater issue of whether or not the bot will record data that doesn't fit into the github data model. In fact, this is getting very close to the blueprints for post-github git development.

A two-tiered PR system, one for the aristocracy (I am not calling it a "meritocracy") and one for the hoi polloi, would represent a clean break from github's "software development egalitarianism" ideology that makes everybody (senior developer and newb alike) use the same PR system.

Honestly, I find the idea exciting. The anythingbot never discussed the creation of a bicameral PR system. This is literally the absolute frontier of innovation in software development.

But this means we're now at the awkward stage where we're flirting with the idea of planning a successor to github...on github.

I expect the universe to divide by zero at any moment.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

I expect the universe to divide by zero at any moment.

Suppose X = (universe / 0). Is (X / X) = 1?

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

On a serious note though, mostly they are practical reasons, and I haven't thoroughly thought through alternatives yet...

Currently the system is structured so that the voting system for PRs (including calculating windows) is completely independent of the system for issue commands. And I like it that way.

However, it also means that there must be some way of passing information from one sub-system to the other if we want one to influence the other. The obvious way to do this was to put it in the db.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

An alternative would be to just make redundant API calls to re-fetch the information, but I think we are already doing to much API calling...

@anythingbot
Copy link

re: doing too much API calling

The other option is to store github data in a proxy (either flat text files or tables in a database)

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 3, 2017

Question: what exactly does the api_cache thing do? Is that intended to serve this purpose?

Also, supposing we switched to webhooks + db. One could imagine a more flexible architecture in which the webhook contacts chaosbot, chaosbot stores events it cares about in a db, and the rest of the system queries/updates the db for relevant information.

@mdcfe
Copy link
Contributor

mdcfe commented Jun 3, 2017

@mark-i-m I'm not sure that an SQL database would be the best fit - maybe a message queue or task queue could be implemented for storing and queuing events, and that could also lead to improved scalability of the bot.

@chaosbot
Copy link
Collaborator

chaosbot commented Jun 6, 2017

/vote close

This issue hasn't been active for a while. To keep it open, react with 👎

@chaosbot
Copy link
Collaborator

chaosbot commented Jun 6, 2017

/vote close This issue hasn't been active for a while. To keep it open, react with 👎

Vote Failed

@chaosbot
Copy link
Collaborator

chaosbot commented Jun 9, 2017

/vote close

This issue hasn't been active for a while. To keep it open, react with 👎

@chaosbot
Copy link
Collaborator

chaosbot commented Jun 9, 2017

/vote close This issue hasn't been active for a while. To keep it open, react with 👎

Vote Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants