Basic process for PRs

[zenspider]

I'm totally fine if we merge if one of us approves a PR, but I'm also open to requiring 2 of us. This issue is here for discussion.

I'd recommend we use PRs as well. I'm not going to process-nazi this up with tons of bureaucracy tho. I'd be fine if something goes straight to master if it is an obvious fix to a recent problem.

[gcv]

👍 on pushing to master to fix obvious problems and regressions. No need to add bureaucracy for simple things.

Let's use good judgment on reviewing PRs. For example: #91 is meaty and has some implications (most notably wrt #86 and #90), so I think we should both look at it and make some decisions on how we'll approach the underlying issue of frame parameter unreliability.

For code we ourselves write: it makes more sense to me to do PRs for larger chunks and have mutual reviews for larger chunks of work. This includes my own long-standing #80, for which there's some real demand.

[zenspider]

Awesome. Sounds like we're on the same page.

I'm gonna do PRs for my own stuff and run it by you. If I break something stupid and simple, I'll push straight to master, but I expect that to be the exception. For some of the bigger reviews, I'll do what I can, but I might punt due to time or lack of personal usage. Happy to defer to others in those cases.

[zenspider]

I prefer rebase merges. Thoughts?

[zenspider]

It looks like @nex3 mostly worked directly on master and did regular merges for PRs. I like a nice flat history where I can do it, but not enough to fight for it. I'm open to suggestion.

[zenspider]

Oh... I also wanted to discuss another bit. I plan on working on this repo, not my fork. Branching+PR model, but directly with this repo rather than having to maintain / sync my own. Thoughts?

[gcv]

I don't have a strong preference with regard to rebase merges vs ordinary merges. I tend to use the latter in general, but in a way which makes the merge itself look like a single bubble on the history (i.e., before merging, a clean rebase from the branch I'm merging into, then perform a git merge --no-ff). That looks clean enough for me on git log --graph output. Definitely not something I'd fight over in any case.

No objections to a branch+PR workflow directly on this repo, but in that case, I'd like us to use a branch naming convention to help keep things organized. Like prefix every branch with the GH username of the person who made it? I know that smacks of Hungarian notation, but it also makes it obvious who's working on what with just an alphabetical branch name sort. I'm open to other suggestions, of course.

[gcv]

The one really important thing to me here is for at least one of us to have high confidence of a branch's overall correctness before merging to master. I really really don't want to break things for people who install from normal (non-stable) MELPA.

[gcv]

@zenspider: I noticed you have a branch with a version bump out. My opinion on tagging stable releases: let’s do that when (1) we add a substantive feature, and (2) we’re reasonably confident it’s stable. Maybe 1-2 weeks of gestation time on master? FWIW, your recent merge counts.

I like the idea of keeping MELPA Stable users on reasonably recent cuts of the code.

[zenspider]

On Sep 15, 2019, at 17:10, gcv ***@***.***> wrote: No objections to a branch+PR workflow directly on this repo, but in that case, I'd like us to use a branch naming convention to help keep things organized. Like prefix every branch with the GH username of the person who made it? I know that smacks of Hungarian notation, but it also makes it obvious who's working on what with just an alphabetical branch name sort. I'm open to other suggestions, of course.

Yep. Totally.

[zenspider]

On Sep 20, 2019, at 09:15, gcv ***@***.***> wrote: @zenspider: I noticed you have a branch with a version bump out. My opinion on tagging stable releases: let’s do that when (1) we add a substantive feature, and (2) we’re reasonably confident it’s stable. Maybe 1-2 weeks of gestation time on master? FWIW, your recent merge counts.

I think 1-2 weeks makes sense. I’m down. I’ve been using this code for months now so I know it works *for me* (except the way it chooses a new perspective after killing), but that’s not saying much.