charmhelpers migration to github
Marco Ceppi
marco.ceppi at canonical.com
Thu Sep 21 12:13:42 UTC 2017
On Thu, Sep 21, 2017 at 7:40 AM Merlijn Sebrechts <
merlijn.sebrechts at gmail.com> wrote:
> What's the reasoning behind feature branches in private repositories? Why
> not just develop on the master of your private repo?
>
This is just one of the many different philosophies for git source control.
It's easier to sync changes from the upstream, so that the master branch in
a fork always mirrors upstream (or any branches from upstream are clean
mirrors). This allows you to basically namespace work on multiple code
paths - new features, quick patches, etc - without having to trample over
yourself. Since branching is so cheap in git, it makes it easy to keep work
organized.
Not to pick (your contribution was great), but to use your last PR as an
example, those could have been two separate branches / pull requests. The
first for the readme fixes and the second for the new feature. Readme
changes and other small patches tend to get merged quicker, while new
features may be my mired down in reviews.
Marco
> 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.
>>
>
> --
> Juju mailing list
> Juju at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju/attachments/20170921/93a93f19/attachment.html>
More information about the Juju
mailing list