Now I feel stupid about my "delete fork and refork" approach to make sure I get a clean master for each new PR.. :)
2017-09-21 17:38 GMT+02:00 Alex Kavanagh <[email protected]>: > > > On Thu, Sep 21, 2017 at 4:32 PM, James Beedy <[email protected]> wrote: > >> 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 :) >> > > I too, use this approach. I treat the master as 'pristine' and do all > work in branches, and if there's an upstream (and there usually is), sync > that to my master, etc. > > >> >> >> > 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. >> > -------------- 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 >> > [email protected] >> > Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm >> an/listinfo/juju >> > >> > >> > ------------------------------ >> > >> > End of Juju Digest, Vol 80, Issue 20 >> > ************************************ >> >> -- >> 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/ > mailman/listinfo/juju > >
-- Juju mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju
