What's the reasoning behind feature branches in private repositories? Why not just develop on the master of your private repo?
2017-09-21 12:12 GMT+02:00 James Hebden <[email protected]>: > Just lending my support for Dmitrii's guidance here. > Clean history is important, especially to those reviewing changes - but > having a clean history isn't as simple as having a single commit in > place when merging code from a private branch into upstream - it's > more important to isolate logical changes into individual branches and > MP those regularly and without remorse. > It's a fairly comfortable and obvious distinction - but an important > one I think when working with MPs. > > Reviewing these two commits in one MP - > "Updated bundled dependencies" > "Corrected PEP8 errors" > > Or one commit > "Updated bundled dependencies and corrected PEP8 errors" > > ...both are going to be a nightmare for the reviewer, especially given > the GitHub UI's inability to sensibly show changes to a single file when > a lot of files have changed, especially over multiple commits. Keeping > MPs short, sweet and logical will make everyone's lives easier when > working with MPs. > > So, If I'm just agreeing with everyone, why am I replying? Well, > primarily just to point everyone at > https://github.com/juju/juju/blob/develop/CONTRIBUTING.md#workflow > > The existing Juju docs seem to get it right and meet everyone's > expectation here. Seems like a good candidate for improving the current > charmhelpers contributing documentation. > > Also, thanks for all the hard work to make this happen! > > On Thu, Sep 21, 2017 at 12:35:56PM +0300, Dmitrii Shcherbakov wrote: > > I think that following the clean history approach is generally good as it > > makes your life easier during debugging and commit message grepping. > > > > Linus' point about > > > > https://www.mail-archive.com/[email protected]. > net/msg39091.html > > "I want clean history, but that really means (a) clean and (b) history." > > > > having both history and keeping it clean is something that we should > > consider but not enforce too much (subjectively) to avoid making it too > > hard to commit changes. In the end, this transition to github is about > > making it easier to contribute, not requiring a person to read a 100-page > > manual on how to annotate and prepare a commit to push a one-liner. > > > > Given that charm-helpers changes are generally small, I think squashing > is > > OK even using github's mechanism. > > > > For large commits, on the other hand, it makes sense to recreate a pull > > request (close a PR, update and push to a fork, create a new PR) or > update > > an existing PR by doing a complex rebase + force push. When there are > > several logical changes per commit it is quite important to keep them > > separate and squashing everything into a single change is essentially > > hiding history. > > > > An analogy would be file compression: yes, you can squeeze files to a > > single compressed blob and make it a bit smaller but then you have to > waste > > CPU cycles to decode it. In the same way, when you are trying to quickly > > grep through a git log you don't want to waste brain cycles on decoding > > unstructured information. > > > > Best Regards, > > Dmitrii Shcherbakov > > > > Field Software Engineer > > IRC (freenode): Dmitrii-Sh > > > > On Thu, Sep 21, 2017 at 12:02 PM, Merlijn Sebrechts < > > [email protected]> wrote: > > > > > It depends on what you want to optimize the development workflow for. I > > > think we need to optimize for easiness because a lot of contributors > will > > > be ops people who generally have a lot less experience with git and > github. > > > I for example have rebased once in my life, and this was only possible > with > > > Alex walking me through the process. > > > > > > *"Fork to private + PR + dirty fix commits" *is an easy workflow that a > > > lot of people are familiar with and that works well with Github. If you > > > want a cleaner commit history, you can always rebase or squash the PR > > > during merge using the Github UI: https://pasteboard.co/GLmTHnf.png. > > > > > > It's not ideal but it's a small price to pay for more contributors.. > > > > > > > > > > > > > > > > > > > > > 2017-09-20 22:10 GMT+02:00 Alex Kavanagh <[email protected] > >: > > > > > >> There's some interesting ideas in there, Dmitrii. Whatever workflow we > > >> end up with needs to be consistent with the other workflow on the juju > > >> namespace on github.com, which I'm guessing is a simple fork to > private > > >> + PR. > > >> > > >> I've used squashed commits on projects in the past, and they do lead > to a > > >> cleaner git history, which is nice; as you say, it's a bit like > gerrit. > > >> Unfortunately, it's not gerrit, so it's difficult (as the bugs > indicate) to > > >> get it to work like gerrit, if you want to preserve the PR history, > yet > > >> keep clean commits. Once it gets to a PR I think you're pretty much > stuck > > >> with the "GitHub way". > > >> > > >> Cheers > > >> Alex. > > >> > > >> On Wed, Sep 20, 2017 at 8:33 PM, Dmitrii Shcherbakov < > > >> [email protected]> wrote: > > >> > > >>> > I'm guessing that the development workflow will be to fork the > repo, > > >>> and do PRs from your own github version? > > >>> > > >>> In short, yes. > > >>> > > >>> 1. fork; > > >>> 2. clone locally; > > >>> 3. set up 2 remotes (1 for rebasing to upstream, 1 for pushing); > > >>> 4. create a branch, commit and push to your fork; > > >>> 5. create a PR. > > >>> > > >>> > I also guess that the contributing guide will need updating (it > talks > > >>> about bzr). > > >>> > > >>> I would also suggest PR workflow-related updates to that doc given > that > > >>> one cannot > > >>> > > >>> git rebase -i HEAD~<n> # amend, squash, add new commits etc. > > >>> and > > >>> git push # to a forked repo > > >>> > > >>> without doing a force push to update a pull request. In my view, it's > > >>> good to keep the commit history clean instead of adding several > commits on > > >>> top without squashing them. Otherwise it quickly turns into: > > >>> > > >>> "an original commit message to make charm-helpers great > > >>> fixup commit to avoid a typo > > >>> hotfix to the fixup > > >>> final fix - will not happen again" > > >>> * closed a PR as a huge rebase is needed > > >>> > > >>> I would propose the following: > > >>> > > >>> > > >>> - using "push -f" only for private branches used for pull requests > > >>> https://help.github.com/articles/using-git-rebase-on-the-com > > >>> mand-line/#pushing-rebased-code-to-github > > >>> <https://help.github.com/articles/using-git-rebase-on- > the-command-line/#pushing-rebased-code-to-github> > > >>> - using git-pull-request: https://github.com/jd/git-pull-request > > >>> which updates PRs with push -f. > > >>> - following this workflow advice about branches: > https://github.com/j > > >>> d/git-pull-request#workflow-advice > > >>> <https://github.com/jd/git-pull-request#workflow-advice> > > >>> > > >>> Rationale: > > >>> > > >>> - https://blog.adamspiers.org/2015/03/24/why-and-how-to-correc > > >>> tly-amend-github-pull-requests/ > > >>> <https://blog.adamspiers.org/2015/03/24/why-and-how-to- > correctly-amend-github-pull-requests/> > > >>> - https://softwareengineering.stackexchange.com/a/263172 > > >>> - https://blog.adamspiers.org/2017/08/16/squash-merging-and-ot > > >>> her-problems-with-github/ > > >>> - https://www.mail-archive.com/[email protected] > > >>> /msg39091.html > > >>> > > >>> > > >>> More info in a gist. > > >>> https://gist.github.com/dshcherb/2c827ae945dc551da3681313294d6783 > > >>> > > >>> > > >>> Coming from the kernel-type patch-sending process ( > > >>> https://lwn.net/Articles/702177/, https://www.mail-archive.com/d > > >>> [email protected]/msg39091.html) and gerrit ( > > >>> https://www.mediawiki.org/wiki/Gerrit/Tutorial#Comparing_patch_sets) > I > > >>> find github's approach with fixup commits a little strange but > > >>> force-pushing with precautions even to a PR branch is not a silver > bullet > > >>> of course. > > >>> > > >>> https://github.com/isaacs/github/issues/997 > > >>> https://github.com/isaacs/github/issues/999 > > >>> > > >>> It would be great to hear some feedback on this topic so that we are > not > > >>> doing anything random and have a common workflow. > > >>> > > >>> Thanks in advance! > > >>> > > >>> Best Regards, > > >>> Dmitrii Shcherbakov > > >>> > > >>> Field Software Engineer > > >>> IRC (freenode): Dmitrii-Sh > > >>> > > >>> On Wed, Sep 20, 2017 at 4:14 PM, Alex Kavanagh < > > >>> [email protected]> wrote: > > >>> > > >>>> Great stuff; I can confirm that I'm in. I'm guessing that the > > >>>> development workflow will be to fork the repo, and do PRs from your > own > > >>>> github version? > > >>>> > > >>>> I also guess that the contributing guide will need updating (it > talks > > >>>> about bzr). I'm happy to do a PR for that if the workflow can be > confirmed > > >>>> :) > > >>>> > > >>>> Cheers > > >>>> Alex. > > >>>> > > >>>> > > >>>> On Wed, Sep 20, 2017 at 12:59 PM, James Page <[email protected] > > > > >>>> wrote: > > >>>> > > >>>>> If you're a part of the charmers team on Launchpad you should now > > >>>>> either have access to approve pull requests + merge or you should > have an > > >>>>> invite to join the team that can do this :-) > > >>>>> > > >>>>> If you don't have one PM me on freenode IRC with your github > username. > > >>>>> > > >>>>> On Wed, 20 Sep 2017 at 11:57 James Page <[email protected]> > wrote: > > >>>>> > > >>>>>> Hi All > > >>>>>> > > >>>>>> Heres a bit of a status update on migration activity: > > >>>>>> > > >>>>>> Code history migration completed > > >>>>>> Travis CI enabled for unit testing and linting with Py 2.7 and 3.4 > > >>>>>> Repo configured to not allow merges until Travis +1's > > >>>>>> > > >>>>>> TODO > > >>>>>> > > >>>>>> Make sure all members of the current team on launchpad are part of > > >>>>>> the charmhelpers team - that should be completed today > > >>>>>> Fixup charmhelpers sync tooling to work from github - this week > > >>>>>> (mainly used by OpenStack Charms team) > > >>>>>> Redirect lp:charm-helpers landings to > github.com/juju/charm-helpers > > >>>>>> > > >>>>>> and the prize goes to Merlin for raising the first non-migration > > >>>>>> related pull request :-) > > >>>>>> > > >>>>>> > > >>>>>> On Tue, 19 Sep 2017 at 14:57 Bryan Quigley < > > >>>>>> [email protected]> wrote: > > >>>>>> > > >>>>>>> From other projects I've seen moved, I'd much prefer if the Code > > >>>>>>> section (and any other sections not planned on being using > anymore) were > > >>>>>>> cleared out on LP and then disabled. > > >>>>>>> > > >>>>>>> Thanks! > > >>>>>>> Bryan > > >>>>>>> > > >>>>>>> On Tue, Sep 19, 2017 at 9:42 AM, Marco Ceppi < > > >>>>>>> [email protected]> wrote: > > >>>>>>> > > >>>>>>>> I've updated the launchpad description to highlight the change. > > >>>>>>>> Since there's bound to be processes still pointing at the lp > branch, should > > >>>>>>>> we set it up as a mirror from git? > > >>>>>>>> > > >>>>>>>> On Tue, Sep 19, 2017 at 9:37 AM James Page < > [email protected]> > > >>>>>>>> wrote: > > >>>>>>>> > > >>>>>>>>> OK - step 1 completed; I've pushed fresh bzr->git migrated > code to > > >>>>>>>>> > > >>>>>>>>> https://github.com/juju/charm-helpers > > >>>>>>>>> > > >>>>>>>>> Please don't land any further changes into the bzr branch as > we'll > > >>>>>>>>> need to diverge from this point forwards. > > >>>>>>>>> > > >>>>>>>>> I will land a commit in lp:charm-helpers to point lost souls to > > >>>>>>>>> the new github.com location as part of the migration. > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>>> On Mon, 18 Sep 2017 at 14:15 Alex Kavanagh < > > >>>>>>>>> [email protected]> wrote: > > >>>>>>>>> > > >>>>>>>>>> I'm a +1 on this too. Let the good times roll. > > >>>>>>>>>> > > >>>>>>>>>> On Mon, Sep 18, 2017 at 11:22 AM, James Page < > > >>>>>>>>>> [email protected]> wrote: > > >>>>>>>>>> > > >>>>>>>>>>> Resurrecting this thread; I think its a good time to push on > > >>>>>>>>>>> with this work - anyone have any objections to targeting > this week to > > >>>>>>>>>>> complete the migration? > > >>>>>>>>>>> > > >>>>>>>>>>> On Fri, 21 Jul 2017 at 19:55 David Ames < > > >>>>>>>>>>> [email protected]> wrote: > > >>>>>>>>>>> > > >>>>>>>>>>>> On Fri, Jul 21, 2017 at 5:46 AM, James Page < > > >>>>>>>>>>>> [email protected]> wrote: > > >>>>>>>>>>>> > Hi All > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > Managed to find some time to test the bzr->git migration > > >>>>>>>>>>>> more, including > > >>>>>>>>>>>> > some tidy of committers and other general hygiene. > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > https://github.com/juju/charm-helpers > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > I think we're in a good position to plan for a switch - I > > >>>>>>>>>>>> appreciate there > > >>>>>>>>>>>> > are a number of open reviews against the bzr branch for > > >>>>>>>>>>>> charmhelpers so it > > >>>>>>>>>>>> > would be nice to get those landed where possible first. > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > I can re-run the process at any time so we can pick when > we > > >>>>>>>>>>>> want to actually > > >>>>>>>>>>>> > switch over. > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > Once we have migrated, we can push forward on travis setup > > >>>>>>>>>>>> etc... so that we > > >>>>>>>>>>>> > can automatically test pull requests. > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > Cheers > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > James > > >>>>>>>>>>>> > > >>>>>>>>>>>> I landed two of Alex's MPs today which fix unit test > failures > > >>>>>>>>>>>> that > > >>>>>>>>>>>> would need to get pulled in. Other than that, the road is > clear > > >>>>>>>>>>>> from > > >>>>>>>>>>>> the OpenStack Charm team. > > >>>>>>>>>>>> > > >>>>>>>>>>>> -- > > >>>>>>>>>>>> David Ames > > >>>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> -- > > >>>>>>>>>>> Juju mailing list > > >>>>>>>>>>> [email protected] > > >>>>>>>>>>> Modify settings or unsubscribe at: > > >>>>>>>>>>> https://lists.ubuntu.com/mailman/listinfo/juju > > >>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> -- > > >>>>>>>>>> Alex Kavanagh - Software Engineer > > >>>>>>>>>> Cloud Dev Ops - Solutions & Product Engineering - Canonical > Ltd > > >>>>>>>>>> > > >>>>>>>>> -- > > >>>>>>>>> Juju mailing list > > >>>>>>>>> [email protected] > > >>>>>>>>> Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailm > > >>>>>>>>> an/listinfo/juju > > >>>>>>>>> > > >>>>>>>> > > >>>>>>>> -- > > >>>>>>>> Juju mailing list > > >>>>>>>> [email protected] > > >>>>>>>> Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailm > > >>>>>>>> an/listinfo/juju > > >>>>>>>> > > >>>>>>>> > > >>>>>>> > > >>>>> -- > > >>>>> Juju mailing list > > >>>>> [email protected] > > >>>>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm > > >>>>> an/listinfo/juju > > >>>>> > > >>>>> > > >>>> > > >>>> > > >>>> -- > > >>>> Alex Kavanagh - Software Engineer > > >>>> Cloud Dev Ops - Solutions & Product Engineering - Canonical Ltd > > >>>> > > >>>> -- > > >>>> Juju mailing list > > >>>> [email protected] > > >>>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm > > >>>> an/listinfo/juju > > >>>> > > >>>> > > >>> > > >> > > >> > > >> -- > > >> Alex Kavanagh - Software Engineer > > >> Cloud Dev Ops - Solutions & Product Engineering - Canonical Ltd > > >> > > >> -- > > >> Juju mailing list > > >> [email protected] > > >> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm > > >> an/listinfo/juju > > >> > > >> > > > > > > -- > > Juju mailing list > > [email protected] > > Modify settings or unsubscribe at: https://lists.ubuntu.com/ > mailman/listinfo/juju > > > -- > James Hebden > Cloud Reliability Engineer > BootStack Squad @ Canonical Ltd. >
-- Juju mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju
