Update example.bashrc to work with Drush 6 changes to "sa" #39
Conversation
|
Thank you for making a branch. Just for the record (and for those who cannot see the Merge pull request control), git's advice for merging from the command line is: git checkout -b jbrauer-update_bashrc master Now, really, what we want to do is insert some testing after the first two commands, before merging the branch into master. I think that the Merge pull request is going to do all of the above instructions in one operation, though, even though you made a branch. I'm going to click it, just to confirm that, even though it means I may have to back out the commits if anything goes wrong. Code looks good, probably that won't be necessary. |
Update example.bashrc to work with Drush 6 changes to "sa"
|
Okay, the code changes worked brilliantly, so this code is accepted into master. Unfortunately, the 'merge pull request' button still merged your patches straight into the master branch, even though you made a feature branch for it. This means that there is little reason for us to request contributors to re-roll on a separate branch if they submit something directly to master, as we can still merge it to a branch from the cli, and the 'merge' button does not work correctly in any event. Take away: never use the merge pull request button. :( Thank you for your contribution. Next time, it would be better to squash all of your changes into a single commit. That makes it easier to review. |
|
I think the workflow is for the reviewer to apply the code locally and test if he wants to (travis already tests all pull requests - once). I made PRs available as remotes in my local clone. I also use Tower to visualize those remotes. Once reviewer is satisfied, he can use web to merge or use CLI commands. Now, we should talk about backporting. These commits need to be backported to 8.x-6.x. I have been using cherry-pick - would be 3 commits in this case. I'm ok with committers using multiple commits instead of squashing if that is clearer. Github's 'Files changed' tab shows a consolidated diff inclusive of all commits. One can use the CLI to do same. |
|
Yes, I already used cherry-pick (three times) to backport this to 8.x-6.x. (Are we going to resolve #16, or stick with legacy drupal.org branch naming conventions?). I've been backporting most of the things I've committed, but I'm not sure if I missed some, or how to track which have been done. I'll try the tools you mention & see if that improves things. |
|
My $.02... I use github a lot... The workflow is different from that on d.o and it's tough to make it the same. This isn't inherently better or worse, just different. One of the big differences is d.o encourages a workflow where a patch comes from each issue. Regardless of how many people contribute and over what period of time there is more or less one canonical patch that comes from an issue. It is, in the end, a "show your final product" sort of environment. GitHub is more of a show your work sort of place. One ends up with a series of changes, probably made on a branch in a fork, that attempt to solve a problem. There, warts and all, are the individual elements that got me to that outcome. It's not as "clean" in the sense that generally there are many commits in the process, even more so if one uses the web interface because each edit is a separate commit. Then a pull request comes in. As @weitzman mentioned the changed files gives you the net effect of the final diff. But this is also where it gets a bit messy. There isn't a clean way to say a pull request is postponed. Sure it can be closed, and perhaps you re-title it to POSTPONED: so it can be found but it's there in the pull request queue till it's closed. And once closed it's in the sea of closed issues not easily distinguishable from closed-fixed. It also means in cases where projects are working across branches that you either ultimately need to merge branches, do a pull request to each branch, or cherry-pick multiple commits. In the alternative when a user is "done" they would need to create a new branch and squash all the changes into a single commit and make the pull request from there. It gets trickier as it goes, however. Two people are working away on the same thing and face a challenge. Each has a fork of the project. So I submit a pull-request related to a particular issue and it fails testing, has a typo, or needs major re-work. In the d.o flow someone else can come along and make these changes and submit an updated patch. On GitHub they face some challenges ... do they fork my fork to get my commits from my branch? Then do a pull request back to my fork to have those changes included? Or maybe the person making the PR is on vacation and not around to follow up? There really doesn't seem to be a good way to collaborate around a pull request. It's possible that I'm missing it (see above I've missed plenty). Also with a couple of long-running pull requests getting intermixed resolving merge conflicts, let alone cherry picking commits to another branch can get dicey. Have a bug and want to figure out where it came from just wait to do a git bisect on a branch with two long-running pr's merged into it. It's a great way to use up a day. By contrast the GitHub process works pretty well in closed groups. Because access can be granted it's easy for me to push a commit or two into someone else's branch and have that considered in the context of the existing pull request. Or you even skip forks entirely and everybody branches in the main repo and do pull requests between branches. |
|
It is likely that milestones could be used to address the "POSTPONED" issue perhaps. |
|
Regarding multiple people working on one pr, although I have not tried it yet, it appears that if you make a feature branch, then everyone who commits to that feature branch will have their commit show up in the pr issue. I'm not sure if this is still the case if the commit is pushed only to the contributor's local fork (e.g. if they do not have push access to the main repository). I think that the "github way" you describe above is fine, except for the issue of backporting an issue with multiple commits. We're in a position right now where our workflow is to commit first to master, and then many commits will be backported to both 6.x and 5.x. I suppose that will not last for too long, and we'll only need to backport to a single older branch, but for the time being it's a little inconvenient. |
|
I think for that to work the feature branch has to be something that everyone has permissions to commit to. As far as I know the only way to do that is have someone in their fork grant others commit access which is fine if the need to collaborate is planned ahead but I'm not sure that's always the case in community projects. |
This replaces #35.