-
Notifications
You must be signed in to change notification settings - Fork 757
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
Document SynthDef.writeOnce as a legacy method #1918
Conversation
What's all this "Merge pull request #1 from supercollider/master …" about? Seems like something weird happened when you created your branch. It makes for some really weird stuff in the commit history. This should have been (re)based directly on master before merging, I think |
Ah ok, I assumed that would be transparent once merged. Big issue or should we just remember this for the next time? |
Sorry about those. I'm bad at git and I did some weird stuff while trying to update snappizz/master to match supercollider/master... |
@telephon I don't really know... |
At this point, the history is in there... Even if you revert, it's still going to look funny. The takeaways are:
|
Or don't fork, hahahaha ;) |
I think this shows that it did boil down to nothing:
|
Hm, Scott C. pointed out that the "merge PR" entry does not appear in the main repo's history. There's a spurious "merge remote tracking branch" entry, but no harm done. But it's still safer to make sure everything looks right before merging permanently. @bagong I maintain that the workflow you proposed elsewhere offers no greater insurance against error than github's recommendation. It's merely a matter of what feels better to you, but it is no safer. |
If you promise not to get angry I'll defend my workflow, with the chance of learning something when I am shown to be wrong ;) All this doesn't matter if you learn how to set default pull and push repos. But as long as you're not there it is safer to keep the branch names of the real origin what they are, and do your own stuff exclusively on branches you actually create yourself (with different names than master etc...) Then pull will always bring in what you really should relate to, the data in the repo your pull request should diff from. The main advantage of "my" workflow: you don't have to manually merge/synch master here and there. I consider that safer (and significantly more practical). I think "fork" is a Github rather than a git thing. That button alongside the badget "forked on Github" was an extremely successful marketing step, but the simplicity with which you can "fork" comes at the price that you don't synchronize automatically anymore with the source repo, and sometimes don't even realize it. This argument that Github saves storage space etc., I don't think it's valid. For one I am not sure I want to save storage space for Github if it makes my workflow more complicated, but also: I am pretty sure the first thing Github does when data is pushed, is to see if a hash is already there, and replace it by a reference if yes. |
With no anger, real or implied: If I understand, you're suggesting that contributors should set "origin" to point to supercollider/supercollider. Then, to make a PR, they should push the topic branch into the main SC repository. For that, some developer has to give every potential contributor explicit write access to the repository. That's an administrative headache, not to mention that it raises the risk of an occasional contributor merging code into the main line without sufficient review. Github's solution is that every potential contributor has her own version of the repository. GH also needs to be sure that the user's repository corresponds to the main one. Currently it does this by forks. I think it will not work if you clone SC and then push your local copy back to your own github account. It "works" for your own code maintenance, but it won't let you open a PR that way. I could be wrong but I don't see a way to do entirely without forks. Whether to follow github's "origin" vs "upstream" nomenclature is a different issue, certainly debatable. |
I agree with Mr. Jam Shark. Dangerous to set origin to supercollider. Keep Though github's hub workflow shows it as keeping origin, Example workflow for contributing to a project:$ git clone github/hub It's time to fork the repo!$ git fork→ (forking repo on GitHub...)→ git remote add YOUR_USER push the changes to your new remote$ git push YOUR_USER feature# open a pull request for the topic branch https://hub.github.com/ https://hub.github.com/ and I guess my origin is supercollider/supercollider So I always pull my changes from there. I push to crucialfelix/supercollider only when I am making a PR I didn't read bagong's git workflow yet. sorry On Wed, Mar 23, 2016 at 4:40 AM jamshark70 notifications@github.com wrote:
|
Chris, you say you're agreeing with James, but I think de facto you are agreeing with me ;) You say it's dangerous to set origin to supercollider, but from all I can see you are doing exactly that ;) I admit, there is a risk of accidentally pushing to supercollider/supercollider if you have write access, but making it a top priority rule to always make your push target explicit reduces that significantly. So "never-never-never":
But always:
@james, it's not really a terminological thing (although I do believe "origin" is misleading and "upstream" is obscure), but it's about where you pull from by default. Of course I don't advocate pushing to supercollider/supercollider, the repo would be a mess in a week. And that's not talking about write access. If we step down further in that attempt to uncover hidden implications, the crucial point, I think, is here:
Well, it will, but it's less obvious. You can press the green "create pull request" button and then choose another repository (supercollider/supercollider) to direct that PR to. And you can even choose the individual branches that are compared in the PR if you press "compare between forks". So forking after all does have it's use, it makes directing a PR a little bit more simple (but it also makes it appear like something "magic", and then people direct the PRs to the wrong branch). So by all means, "fork", but do it like Chris ;), only use the fork to push to, but not as your default pull "origin", otherwise you'll have to manually sync your master and supercolliders master, which is very error prone. The messy situation starts when people clone their own fork and then find it doesn't sync with supercollider/supercollider automagically. If you clone supercollider/supercollider and then learn how to push to your own repo that sync problem never occurs. |
Yes, I know my origin is set to supercollider. I make sure never to push to
Like when Julian was trying out the github desktop client and it f*#$%ing On Wed, Mar 23, 2016 at 10:27 AM Rainer Schütz notifications@github.com
|
@bagong let me see if I understand your approach:
If I understand correctly, I have no specific objection to the procedure. I question some of the discussion of it:
|
Yes, @jamshark70 , thanks for spelling out clearly and succinctly. I could try to split a hair on what a fork actually is, and what to weigh stronger, errors that occur every day 5 times vs. errors that are pretty rare (but dangerous), but basically I agree to every point you make. |
As discussed in #1910.