Skip to content
This repository has been archived by the owner on Feb 2, 2021. It is now read-only.

How do I contribute a CL? #1970

Open
erights opened this issue Jul 5, 2015 · 3 comments
Open

How do I contribute a CL? #1970

erights opened this issue Jul 5, 2015 · 3 comments

Comments

@erights
Copy link
Contributor

erights commented Jul 5, 2015

https://github.com/google/caja/wiki/ContributingCode is obsolete. It still speaks in terms of myvn which is no longer relevant since the switch to github.

@kpreid
Copy link
Contributor

kpreid commented Jul 20, 2015

Okay, so we need to update the instructions and create a replacement for myvn. It is possible to use tools/appspot.py directly to do code reviews.

I have been carefully executing a minimum-changes-from-svn workflow, but I'm open to suggestions as to making it more git-ish. The things I didn't want to lose, but don't fit all that well with standard git workflows, is the code review information in the message for every commit, and the linearity of history.

In the absence of such changes, here is what a new myvn-analogue would do, I think:

  • myvn change replacement: same thing. However, it would specifically refuse to operate on a branch that is not a feature branch (we could enforce a naming convention, or recognize the names master and es53).
  • myvn snapshot: same, but ensure all changes are committed (this is not strictly necessary, but is a good work habit).
  • myvn submit replacement, part 1: would take the current history of the feature branch and squash it into one commit, with a message derived from the appspot-change file (and warn you that it's doing this to give you a chance to edit the change file).
  • myvn submit replacement, part 2: validate that the history is in the above form, run tests, then optionally put it in master (effectively as a cherry-pick) and optionally push.

This would be sufficient to handle the workflow I've been doing manually since the migration. The docs would be updated to explain the above.

I specifically do not want to do GitHub pull requests by themselves because from what I've seen, Rietveld is superior to GitHub's code review support, and I find a lot of value in avoiding unnecessarily nonlinear history. However, we should be able to accept contributions which are in the form of pull requests, which suggests a new myvn feature:

  • Take a pull request or other branch that's already been fetched and bring it into the workflow.

@jfparadis
Copy link
Contributor

Keith:

I've submitted three pull requests (and more are coming). However, I just assumed a standard process, and just noticed last night this note about using an analogous process to myvn.

Would you prefer to get those three pull requests in a single one? Anything other request?

@kpreid
Copy link
Contributor

kpreid commented Aug 30, 2016

jfparadis, pull requests are fine for little things from non-members — just don't expect them to get merged unmodified.

Regarding your own pull requests, please sign the Google CLA like the bot comment said.

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

No branches or pull requests

3 participants