| zxiiro | Liskni_si: Gerrit's kind of the complete opposite workflow to GitHub PRs where you revise single independent patches until they merge. | 06:22 |
|---|---|---|
| zxiiro | it's a bit confusing if you're used to the github workflow hehe | 06:23 |
| zxiiro | I made it to Amsterdam despite the tornado in my city on the way out :) | 06:23 |
| Liskni_si | zxiiro: So what does that mean for me? That I'm supposed not to use branches and just submit squashed stuff? No thanks. :-) | 08:06 |
| zxiiro | Liskni_si: the workflow for Gerrit is "git commit --amend" your patches that you've already pushed online. | 08:06 |
| Liskni_si | I'm doing just that, I have no problem with this. | 08:06 |
| Liskni_si | And I suspect that with git range-diff, that will become the workflow everywhere else as well. | 08:07 |
| zxiiro | Liskni_si: Gerrit's workflow is very much like the Linux Kernel's workflow. we don't merge patches until they are perfect. Unlike GitHub where individual commits are meaningless and instead they use merge commits to bundle patches together as a PR | 08:07 |
| Liskni_si | zxiiro: That's just not true. | 08:08 |
| Liskni_si | zxiiro: "pull request" comes from the Linux kernel workflow. | 08:08 |
| zxiiro | Liskni_si: not GitHub's model of it | 08:08 |
| Liskni_si | zxiiro: I don't think so. Why? | 08:09 |
| zxiiro | Liskni_si: https://github.com/torvalds/linux/pull/17#issuecomment-5654674 | 08:09 |
| zxiiro | Liskni_si: Linus posted this in 2012 but it still holds true today :) | 08:09 |
| zxiiro | Liskni_si: Yes the linux kernel uses PRs however GitHub's implementation of PRs is not the model the Linux Kernel project recommends, nor uses. | 08:09 |
| Liskni_si | zxiiro: Yeah, we all dislike the github UI, but the concept of the PR is still the same. | 08:10 |
| zxiiro | Liskni_si: it is not, read that thread I posted, it is from Linus Torvalds himself and he explains why GitHub PRs are not the same. | 08:10 |
| Liskni_si | A bunch of commits one after another that may or may not be related, every which one should build and pass tests. | 08:10 |
| Liskni_si | I'm reading the thread you posted, and I'm still not seeing how my commits are affected. | 08:12 |
| zxiiro | Liskni_si: GitHub is a great service as long as you are not doing things through the UI. and you _can_ get the Linux kernel's PR model even with GitHub as long as you completely avoid using GitHub's UI and purely use GitHub as a repo service only. and send PRs via a different process. | 08:12 |
| zxiiro | Liskni_si: I'm not talking about your commits. | 08:12 |
| zxiiro | Liskni_si: I thought we were just talking about PRs in general. | 08:12 |
| Liskni_si | zxiiro: If you're not talking about my commits, why are we having this conversation? | 08:12 |
| zxiiro | Liskni_si: Sorry I thought we were just bantering about code submission models | 08:13 |
| zxiiro | Liskni_si: I wasn't directly commenting about your contributions. I happened to see you and ssbarnea|bkp talking about it on one of the patches. | 08:13 |
| zxiiro | Liskni_si: friendly chat, apologies if it sounded like I was commenting about your contributions directly. | 08:14 |
| Liskni_si | Sorin asked me to split my submissions into changesets that don't depend on one another, because that'd make merging them separately easier/possible. But I never wanted to merge them separately, and I find value in discussing the branch as a whole. Gerrit makes it hard to discuss the branch as a whole, just as Github makes it hard to discuss individual commits. I'd love to be able to discuss | 08:15 |
| Liskni_si | both. :-) | 08:15 |
| zxiiro | right, Gerrit "sort of" the github PR model via "topic-branches" as the way you are doing them. | 08:16 |
| zxiiro | but it's workflow is definitely highly patchset depending as opposed to branching and multi-commits. | 08:17 |
| zxiiro | Just hard to discuss as you say | 08:19 |
| Liskni_si | Yeah. I must admit that anything that might even slightly turn people towards squashing patches is a sensitive topic for me. Sorry. :-) | 08:20 |
| Liskni_si | I've seen too many 1000+ line commits by colleagues from different teams and my reaction is usually screaming and such. | 08:22 |
| zxiiro | right you shouldn't have 1000+ line commits. and Gerrit does not recommend that either. Gerrit wants you to make a lot of small commits that can be merged by itself though. | 08:24 |
| zxiiro | like if you are adding a new feature, instead of doing the complete feature, in the Gerrit model you might have a commit that adds the API and the tests for the API, and then in a 2nd commit utilize the API. | 08:25 |
| zxiiro | Gerrit stats from Google suggest that patches that are more than 500+ lines of code tend to no get reviewed. | 08:25 |
| zxiiro | order matters in the Gerrit model. | 08:26 |
| Liskni_si | It's true that I quite often submit unrelated cleanups in the same github pull request because I'm too lazy to create a separate branch/pr for it. But I don't immediately see whether gerrit would help with my laziness. I suspect not. :-) | 08:27 |
| Liskni_si | (But I don't think I've done this in the tja-fast-get-jobs topic.) | 08:27 |
| zxiiro | Liskni_si: unfortunately not, in the Gerrit model you'd make a completely different patch / branch for that which from a lazy person's perspective would be more work for you. | 08:27 |
| zxiiro | the git review tool that the openstack-infra folks have been developing is very useful for lazyness though | 08:28 |
| zxiiro | it manages branches for you | 08:28 |
| Liskni_si | gerrit would definitely help with getting that patch reviewed, though. Part of my laziness comes from the need to find reviewers for a PR, and gerrit lists all commits as equal, so submitting a separate branch doesn't create additional work finding reviewers. | 08:29 |
| zxiiro | I very regularly do "git commit && git review" and then delete my branch because I know when I want to work on a patch again "git review -d <review>" will automatically manage fetching and branching for me. | 08:29 |
| zxiiro | right. sadly reviewers is a big issue for many of the projects i work on | 08:30 |
| Liskni_si | Yeah. I recently adopted a similar workflow with github, supported by my own tool that fetches and checks out a pr branch. | 08:30 |
| zxiiro | it's very hard to find people who have the time to regularly stay ontop of reviews. especially if they are just volunteering their time. | 08:30 |
| Liskni_si | I might try using it more often to submit unrelated stuff separately, and by that I may become a better gerrit user as well. :-) | 08:30 |
| zxiiro | which is the case for JJB / python-jenkins. we don't have people being paid full time to work on these tools. | 08:31 |
| Liskni_si | But they're paid at least part-time, no? | 08:31 |
| zxiiro | Liskni_si: yes, I very highly recommend getting familiar with git-review. it lets you be more lazy when working with Gerrit :) | 08:31 |
| Liskni_si | I think I use git-review. How would I update those amended patches otherwise? :-) | 08:32 |
| zxiiro | Liskni_si: I don't know about the other folks, but the only reason I work on JJB / python-jenkins is because we (The Linux Foundation) depend on these tools. so I am part-timed paid to maintain these tools but it is not my main folks, and our only interest in JJB is to ensure it works for us. | 08:33 |
| zxiiro | main focus* | 08:33 |
| Liskni_si | Yeah, that's probably the same with me and hrubi as well, and some guys from RedHat as well, right? | 08:34 |
| zxiiro | yep. exactly. | 08:34 |
| zxiiro | python-jenkins isn't actually that interesting to us other than because JJB uses it. I just want to make sure contributions to that doesn't break our infra. | 08:35 |
| zxiiro | sadly we've been broken by python-jenkins changes more than a few times :( | 08:35 |
| Liskni_si | We have some additional logic like job renaming and disabling based on the diff between old/new JJB outputs whenever changes to job definitions are merged, which could be implemented using python-jenkins, but is actually implemented in bash using curl because we still haven't figured out a way to manage python dependencies that we'd be happy with. :-) | 08:38 |
| openstackgerrit | Anil Belur proposed openstack-infra/jenkins-job-builder master: cucumber-reports: Add support for new options https://review.openstack.org/599218 | 11:51 |
| openstackgerrit | Anil Belur proposed openstack-infra/jenkins-job-builder master: jclouds: Utilize convert_mapping_to_xml https://review.openstack.org/602615 | 11:54 |
| openstackgerrit | Anil Belur proposed openstack-infra/jenkins-job-builder master: docker-pull-image: Pull Docker image from Docker Hub/Registry https://review.openstack.org/602982 | 12:12 |
| openstackgerrit | Anil Belur proposed openstack-infra/jenkins-job-builder master: Add support for "Build / Publish Docker Image" https://review.openstack.org/601291 | 12:30 |
| ssbarnea|bkp | haha, funny to read Liskni_si comment, indeed python lib deps can be tricky for non python devs, many tools affected by it. | 16:19 |
| Liskni_si | ssbarnea|bkp: Hi, if you have a specific suggestion which patches might be worth submitting independently, I'd like to hear that. Because I still see my branch as one whole and I'd wish for it to appear as one merged branch in git log --format=oneline --graph. Having said that, I don't intend to go finding ways to split it, and if you already see those ways, why not tell me. :-) | 17:50 |
| ssbarnea|bkp | Liskni_si you need to keep each of your change in a different branch. | 17:51 |
| ssbarnea|bkp | you can ignore my recommendation but just don't be surprised if it would take a lot of time to merge your changes, by the time you fix the last, the first change in the chain may be broken,... | 17:52 |
| ssbarnea|bkp | it was just a hint, i learnt this the hard way. | 17:52 |
| Liskni_si | ssbarnea|bkp: Keeping every change in a separate branch is a technicality. What I meant was "tell me which patches out of those 6 can a should be separate, pls". They all touch the same tests, so they actually are dependent on each other. Maybe some of those will merge just fine. Maybe some will merge almost fine with some hand-resolvable conflicts. Not worth it, imo. | 18:00 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!