Hi Kevin, Yep, Review Board will verify each file in the diff to make sure the files can be found, preventing a category of problems at diff review time. (It doesn't verify whether the files can be cached, just does an existence check on each file, which has varying degrees of complexity depending on the hosting service or type of repository.)
By default, we don't even allow for diffs over 1MB, because of the performance issues and the inability for humans to effectively review changes of that size (leading to "Eh, Ship It, I have other things to do" reactions, and thus basically unreviewed code). Christian -- Christian Hammond President/CEO of Beanbag <https://www.beanbaginc.com/> Makers of Review Board <https://www.reviewboard.org/> On Mon, Mar 20, 2017 at 2:03 PM, Kevin Yu <[email protected]> wrote: > Hi Christian, > > Thanks for taking the time to look at this issue. > > From what I have seen so far, after rbt diff, when i run rbt post to post > this diff to reviewboard, I am guessing reviewboard has checking as well? > When I run rbt post, it took very long( I cancelled after 10 hours) to > post. During that time, I did a tcpdump to verify whether the process died > in the middle or it really takes this long. From what i can see in the > tcpdump, there's still packet transfer between my local and reviewboard > server. > > > On Friday, March 17, 2017 at 11:37:59 PM UTC-7, Christian Hammond wrote: >> >> Hi Kevin, >> >> 5 hours, wow. Well, the reason this is happening is that `svn diff` >> itself isn't very reliable for some cases, and we have to fix up some >> metadata in the diff. This means going through each file and doing a lookup >> and rebuilding some diff data. That's a lot of work. It doesn't add too >> much to the time for standard changes, but a whole source tree, definitely. >> >> Nobody can realistically review a change of that size, and the best thing >> to do is leave that directory out of the diff being posted (using the -X >> argument, for exclude). People can be told through the review request >> description that it's been deleted instead. >> >> Ideally, though, we wouldn't be this slow. Ideally we could just trust >> the diff we're given. There might be things we could do to simplify the >> work needed for changes of this size, and maybe there's some good >> optimizations we can do that would help everyone. 750MB is definitely a >> corner case, though. Still, I want to track this, see if it leads to >> possible shortcuts/improvements in our algorithm. >> >> Christian >> >> >> On Fri, Mar 17, 2017 at 16:15 Kevin Yu <[email protected]> wrote: >> >>> I recently run in this issue. Basically I had to svn rm an older kernel >>> folder which is about 750MB. I did a rbt diff but it's not return after 5 >>> hours and counting. >>> >>> I understand it's not normal practice to delete a huge folder and post >>> to reviewboard. However, is this performance expected? >>> >>> -- >>> Supercharge your Review Board with Power Pack: >>> https://www.reviewboard.org/powerpack/ >>> Want us to host Review Board for you? Check out RBCommons: >>> https://rbcommons.com/ >>> Happy user? Let us know! https://www.reviewboard.org/users/ >>> --- >>> You received this message because you are subscribed to the Google >>> Groups "reviewboard" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> For more options, visit https://groups.google.com/d/optout. >>> >> -- >> -- >> Christian Hammond >> President/CEO of Beanbag <https://www.beanbaginc.com/> >> Makers of Review Board <https://www.reviewboard.org/> >> > -- Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/ Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/ Happy user? Let us know! https://www.reviewboard.org/users/ --- You received this message because you are subscribed to the Google Groups "reviewboard" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
