charmhelpers migration to github

James Beedy jamesbeedy at gmail.com
Thu Sep 21 15:32:10 UTC 2017


Tracking many upstream projects, I find it most clean to just fetch the the upstream master once my feature branch PR has landed, such that I don't push or merge anything to my own master branch. (Ideally) I only use master branch to sync with upstream and to make new branches from. But that's just me :)


> 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 <james.hebden at canonical.com>:
> 
>> 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/dri-devel@lists.sourceforge.
>> 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 <
>>> merlijn.sebrechts at gmail.com> 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 <alex.kavanagh at canonical.com
>>> :
>>>> 
>>>>> 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 <
>>>>> dmitrii.shcherbakov at canonical.com> 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/dri-devel@lists.sourceforge.net
>>>>>>  /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
>>>>>> ri-devel at lists.sourceforge.net/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 <
>>>>>> alex.kavanagh at canonical.com> 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 <james.page at ubuntu.com
>>> 
>>>>>>> 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 <james.page at ubuntu.com>
>> 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 <
>>>>>>>>> bryan.quigley at canonical.com> 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 <
>>>>>>>>>> marco.ceppi at canonical.com> 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 <
>> james.page at ubuntu.com>
>>>>>>>>>>> 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 <
>>>>>>>>>>>> alex.kavanagh at canonical.com> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>>> I'm a +1 on this too.  Let the good times roll.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> On Mon, Sep 18, 2017 at 11:22 AM, James Page <
>>>>>>>>>>>>> james.page at ubuntu.com> 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 <
>>>>>>>>>>>>>> david.ames at canonical.com> wrote:
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> On Fri, Jul 21, 2017 at 5:46 AM, James Page <
>>>>>>>>>>>>>>> james.page at ubuntu.com> 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
>>>>>>>>>>>>>> Juju at lists.ubuntu.com
>>>>>>>>>>>>>> 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
>>>>>>>>>>>> Juju at lists.ubuntu.com
>>>>>>>>>>>> Modify settings or unsubscribe at:
>> https://lists.ubuntu.com/mailm
>>>>>>>>>>>> an/listinfo/juju
>>>>>>>>>>> 
>>>>>>>>>>> --
>>>>>>>>>>> Juju mailing list
>>>>>>>>>>> Juju at lists.ubuntu.com
>>>>>>>>>>> Modify settings or unsubscribe at:
>> https://lists.ubuntu.com/mailm
>>>>>>>>>>> an/listinfo/juju
>>>>>>>> --
>>>>>>>> Juju mailing list
>>>>>>>> Juju at lists.ubuntu.com
>>>>>>>> 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
>>>>>>> Juju at lists.ubuntu.com
>>>>>>> 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
>>>>> Juju at lists.ubuntu.com
>>>>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm
>>>>> an/listinfo/juju
>> 
>>> --
>>> Juju mailing list
>>> Juju at lists.ubuntu.com
>>> Modify settings or unsubscribe at: https://lists.ubuntu.com/
>> mailman/listinfo/juju
>> 
>> 
>> --
>> James Hebden
>> Cloud Reliability Engineer
>> BootStack Squad @ Canonical Ltd.
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <https://lists.ubuntu.com/archives/juju/attachments/20170921/942d8633/attachment-0001.html>
> 
> ------------------------------
> 
> Subject: Digest Footer
> 
> -- 
> Juju mailing list
> Juju at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju
> 
> 
> ------------------------------
> 
> End of Juju Digest, Vol 80, Issue 20
> ************************************



More information about the Juju mailing list