Skip to content
This repository was archived by the owner on Jun 1, 2022. It is now read-only.

Unclear what "internal pull request" is #10

Open
benjaoming opened this issue Sep 21, 2016 · 10 comments
Open

Unclear what "internal pull request" is #10

benjaoming opened this issue Sep 21, 2016 · 10 comments

Comments

@benjaoming
Copy link

Internal PRs are mentioned twice in the list:

  1. External API changes and significant modifications ought to be subject to an internal pull-request to solicit feedback from other contributors.
  2. Internal pull-requests to solicit feedback are encouraged for any other non-trivial contribution but left to the discretion of the contributor.

But IMO it's unclear what exactly they are. Also talking about "external API" at the same time makes it slightly more fuzzy language-wise.

Is there perhaps a better term for this?

@strugee
Copy link

strugee commented Apr 30, 2017

I don't think there's a better term (at least not that I can think of). But perhaps we could define it with a footnote or something?

@strugee
Copy link

strugee commented Apr 30, 2017

What an "internal Pull Request" is, by the way, is where you open a Pull Request even though you have the rights to merge the relevant branches. This allows other project contributors to review your code in the same way that an "external" contribution would be reviewed, which is a Good Thing™ because it increases overall code quality.

@benjaoming
Copy link
Author

Ah okay, I understand what you mean.

"All changes should go through Pull Requests - even owners of the repository should open Pull Requests for all their changes"

I think one consequence is that owners of a repository should always work in their own private fork in order to avoid leaving branches around in the original main repository..

@strugee
Copy link

strugee commented May 2, 2017

Why is it bad to leave branches around?

@benjaoming
Copy link
Author

If those branches are visible to others, they will clutter their view. Features and bug fixes happen through branches that aren't relevant after they've been merged - often the branches are forgotten and left dangling in remote repos. That's why it's better to store feature and bug fix branches in your own repo than in a shared organization's repo.

@strugee
Copy link

strugee commented May 2, 2017

@benjaoming that feels like premature optimization to me, for a couple reasons:

  1. GitHub branch views differentiate between "active" and "stale" branches
  2. git branch only shows local branches, not branches fetched from remotes, so it'll only show other peoples' branches that you've explicitly checked out locally (in which case it seems useful for those to be displayed anyway)
  3. Pull Requests prompt you to delete branches after merge, so that helps clean things up too

@benjaoming
Copy link
Author

benjaoming commented May 2, 2017

@strugee
I think I've sidetracked the original decision and whether you agree that it's easier to write what we mean than to introduce and define an unknown term?

As for the other stuff, it's a Github/Git UI thing and you're right that organizations can have branches in their main repos. I just see a lot of orgs getting it wrong, myself included :) But I don't think it's possible to categorically rule out the possibility that a lot of people and maintainers get it right!

screenshot from 2017-05-02 19-38-17

@cben
Copy link

cben commented May 4, 2017

Do all "open open source" projects even have a github org?
Because if you just work in $you/$repo, it will include your feature branches (at least for your active PRs) by definition.
(which is one of the reasons creating an org up front is not a bad idea. but I don't see that mentioned anywhere. is there any separate "how to make your project open open source" guidance?)

@benjaoming
Copy link
Author

@cben maybe it's better to address it separately, since perhaps it's clouding the other decision :)

@strugee
Copy link

strugee commented May 12, 2017

I think I've sidetracked the original decision and whether you agree that it's easier to write what we mean than to introduce and define an unknown term?

Ah, sorry.

In general I think it's easier to introduce and define the term, since while this isn't a super common term (like just plain "Pull Request" is), it's not something this project made up either. That being said I don't really feel strongly about this.

I also forgot about the branch view you posted a screenshot of - nice catch :)

Forked that discussion to #10.

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

No branches or pull requests

3 participants