Re: Review performance issue
Thanks Christian, I will disable syntax highlighting and will watch the performance and let you know the status. Thanks, Mohan. On Mon, Oct 18, 2010 at 12:26 AM, Christian Hammond chip...@chipx86.comwrote: Please disable syntax highlighting (Admin UI - Settings - Diff viewer) and see if that helps. Apache *will* use a lot of CPU when generating a side-by-side diff for a large file. There's a lot of processing that must take place for generating a diff. It's just an unavoidable thing. But it shouldn't be causing massive performance problems. Provided things are cached, it should get better. We know of Review Board being used at many companies, VMware included, with thousands of users and frequent review request creation/diff updates. The spike in CPU usage hasn't caused a problem. Is it that performance is really terrible, or just that you're seeing spikes if you watch the processes? Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.reviewboard.org VMware, Inc. - http://www.vmware.com On Sun, Oct 17, 2010 at 8:45 AM, bugfree o3j5h...@gmail.com wrote: nah. not centos. I'm testing on a gentoo too and see similar issues. restart apache: ps -eo pmem,pcpu,rss,vsize,args returns 1.1 0.0 23588 39108 /usr/sbin/apache2 -D DEFAULT_VHOST ... for every apache process. If I open a diff for a file ~5Mb file one apache process jumps to 16.0 0.0 332192 353328 /usr/sbin/apache2 -D DEFAULT_VHOST ... while the others stays the same. I go back to the dashboard and reopen the very same diff I end up with 16.0 0.0 332192 353328 /usr/sbin/apache2 -D DEFAULT_VHOST ... 16.0 0.0 331640 352876 /usr/sbin/apache2 -D DEFAULT_VHOST ... %MEM %CPU RSSVSZ COMMAND 1.8 0.0 38740 119912 /usr/bin/memcached -d -p 11211 -U 11211 -l 127.0.0.1 -m 512 -c 1024 -u memcached -P /var/run/memcached/ memcached-11211.pid -r -I 4 1.1 0.0 23588 39108 /usr/sbin/apache2 -D DEFAULT_VHOST ... and so on. on the third view of the same diff a third process grows. will test mod_wsgi and let you know. thanks On Oct 15, 9:34 pm, Jan Koprowski jan.koprow...@gmail.com wrote: It looks like the issue is CentOS. 1) Try to switch from mod_python to mod_wsgi (or vice versa) 2) Turn of memcache check is issue gone (switch to file cache) I belive this is good reason to send ticket to CentOS team. On Fri, Oct 15, 2010 at 4:01 PM, bugfree o3j5h...@gmail.com wrote: Same issue here. I'm on centos with RB 1.5. My source files can be as big as 20Mb which I didn't really expect to be an issue. turns out viewDiff can take several minutes. but performance aside I think I'm looking at some memory leakage. after running it for a few days while no user is connected running ps -eo pmem,pcpu,rss,vsize,args | sort -k 1 -r | less is giving me 16.5 0.0 343748 37 /usr/sbin/apache2 -D DEFAULT_VHOST -D INFO -D SSL -D SSL_DEFAULT_VHOST -D LANGUAGE -D PYTHON -d /usr/lib/apache2 -f / etc/apache2/httpd.conf -k start 16.0 0.0 331968 362408 /usr/sbin/apache2 -D DEFAULT_VHOST -D INFO -D SSL -D SSL_DEFAULT_VHOST -D LANGUAGE -D PYTHON -d /usr/lib/apache2 -f / etc/apache2/httpd.conf -k start 15.8 0.0 327960 355388 /usr/sbin/apache2 -D DEFAULT_VHOST -D INFO -D SSL -D SSL_DEFAULT_VHOST -D LANGUAGE -D PYTHON -d /usr/lib/apache2 -f / etc/apache2/httpd.conf -k start 15.4 0.0 321124 371132 /usr/sbin/apache2 -D DEFAULT_VHOST -D INFO -D SSL -D SSL_DEFAULT_VHOST -D LANGUAGE -D PYTHON -d /usr/lib/apache2 -f / etc/apache2/httpd.conf -k start 10.4 0.0 216932 363664 /usr/sbin/apache2 -D DEFAULT_VHOST -D INFO -D SSL -D SSL_DEFAULT_VHOST -D LANGUAGE -D PYTHON -d /usr/lib/apache2 -f / etc/apache2/httpd.conf -k start 6.4 0.0 134208 372964 /usr/sbin/apache2 -D DEFAULT_VHOST -D INFO -D SSL -D SSL_DEFAULT_VHOST -D LANGUAGE -D PYTHON -d /usr/lib/apache2 -f / etc/apache2/httpd.conf -k start 4.1 0.0 85308 203736 /usr/sbin/apache2 -D DEFAULT_VHOST -D INFO -D SSL -D SSL_DEFAULT_VHOST -D LANGUAGE -D PYTHON -d /usr/lib/apache2 -f / etc/apache2/httpd.conf -k start 1.8 0.0 38676 119912 /usr/bin/memcached -d -p 11211 -U 11211 -l 127.0.0.1 -m 512 -c 1024 -u memcached -P /var/run/memcached/ memcached-11211.pid -r -I 4 0.8 0.0 18592 50876 /usr/sbin/apache2 -D DEFAULT_VHOST -D INFO -D SSL -D SSL_DEFAULT_VHOST -D LANGUAGE -D PYTHON -d /usr/lib/apache2 -f / etc/apache2/httpd.conf -k start 0.7 0.0 15656 362552 /usr/sbin/apache2 -D DEFAULT_VHOST -D INFO -D SSL -D SSL_DEFAULT_VHOST -D LANGUAGE -D PYTHON -d /usr/lib/apache2 -f / etc/apache2/httpd.conf -k start 0.1 0.0 3884 242392 /usr/sbin/mysqld --defaults-file=/etc/mysql/ my.cnf --basedir=/usr --datadir=/export/mysql --pid-file=/var/run/ mysqld/mysqld.pid --socket=/var/run/mysqld/mysqld.sock 0.0 0.0 1196 5484 /usr/sbin/vmtoolsd 0.0 0.0 1136 7360
Upgrading 1.0rc3 to 1.5 and Boolean fields queries
I am trying to upgrade our RB from 1.0rc3 to 1.5. We are running on top of a SQLAnywhere database server. I got the database upgraded successfully, but when I try to access the site it gives me errors regarding badly generated SQL queries. The problem is with the reviews_review.public field. This field is a Boolean field. The query that is generated by RB looks as follows: SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public AND ... In the SQLAnywhere Django driver, Boolean fields are mapped to BIT fields on the database side. As such, the correct query above should have been SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public = 1 AND ... I am inheriting this system so I am still learning both Django and RB. What component is responsible for generating the correct syntax for Boolean fields? -- Mohammed -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en
Print diff for one file per page
Hi, Is there any issue to provide optional paginating of diffs. Today some people from my company ask about File per site. Is there issue request for this? This could help reviewing really large diffs. -- Jan Koprowski -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en
Re: Upgrading 1.0rc3 to 1.5 and Boolean fields queries
Hi Mohammed, This query is generated in both reviews/managers.py and reviews/datagrids.py. In these cases, it's raw SQL, so you should be able to easily modify that. Preferably, we'd move away from raw SQL now that Django has support for F() expressions (things like field_name = field_name + 1 and stuff, which it couldn't do before). But a quick fix would be to just do the = 1 on those queries. Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.reviewboard.org VMware, Inc. - http://www.vmware.com On Mon, Oct 18, 2010 at 8:01 AM, Mohammed Abouzour [Sybase] abouz...@gmail.com wrote: I am trying to upgrade our RB from 1.0rc3 to 1.5. We are running on top of a SQLAnywhere database server. I got the database upgraded successfully, but when I try to access the site it gives me errors regarding badly generated SQL queries. The problem is with the reviews_review.public field. This field is a Boolean field. The query that is generated by RB looks as follows: SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public AND ... In the SQLAnywhere Django driver, Boolean fields are mapped to BIT fields on the database side. As such, the correct query above should have been SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public = 1 AND ... I am inheriting this system so I am still learning both Django and RB. What component is responsible for generating the correct syntax for Boolean fields? -- Mohammed -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en
Re: Upgrading 1.0rc3 to 1.5 and Boolean fields queries
Hi Christian, Thanks, that did solve my problem ... but it made me wonder how other backend drivers, namely Oracle, handled this SQL without = 1 ? -- Mohammed On Oct 18, 3:31 pm, Christian Hammond chip...@chipx86.com wrote: Hi Mohammed, This query is generated in both reviews/managers.py and reviews/datagrids.py. In these cases, it's raw SQL, so you should be able to easily modify that. Preferably, we'd move away from raw SQL now that Django has support for F() expressions (things like field_name = field_name + 1 and stuff, which it couldn't do before). But a quick fix would be to just do the = 1 on those queries. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.reviewboard.org VMware, Inc. -http://www.vmware.com On Mon, Oct 18, 2010 at 8:01 AM, Mohammed Abouzour [Sybase] abouz...@gmail.com wrote: I am trying to upgrade our RB from 1.0rc3 to 1.5. We are running on top of a SQLAnywhere database server. I got the database upgraded successfully, but when I try to access the site it gives me errors regarding badly generated SQL queries. The problem is with the reviews_review.public field. This field is a Boolean field. The query that is generated by RB looks as follows: SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public AND ... In the SQLAnywhere Django driver, Boolean fields are mapped to BIT fields on the database side. As such, the correct query above should have been SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public = 1 AND ... I am inheriting this system so I am still learning both Django and RB. What component is responsible for generating the correct syntax for Boolean fields? -- Mohammed -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know athttp://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en
Re: Upgrading 1.0rc3 to 1.5 and Boolean fields queries
Hi, To my knowledge, nobody is using Oracle. rb-site doesn't provide it as an option, and we don't have a license with which to test. Most people so far have been using MySQL and PostgreSQL. Is the Sybase driver becoming public, and will it be part of Django in the future? Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.reviewboard.org VMware, Inc. - http://www.vmware.com On Mon, Oct 18, 2010 at 1:49 PM, Mohammed Abouzour [Sybase] abouz...@gmail.com wrote: Hi Christian, Thanks, that did solve my problem ... but it made me wonder how other backend drivers, namely Oracle, handled this SQL without = 1 ? -- Mohammed On Oct 18, 3:31 pm, Christian Hammond chip...@chipx86.com wrote: Hi Mohammed, This query is generated in both reviews/managers.py and reviews/datagrids.py. In these cases, it's raw SQL, so you should be able to easily modify that. Preferably, we'd move away from raw SQL now that Django has support for F() expressions (things like field_name = field_name + 1 and stuff, which it couldn't do before). But a quick fix would be to just do the = 1 on those queries. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.reviewboard.org VMware, Inc. -http://www.vmware.com On Mon, Oct 18, 2010 at 8:01 AM, Mohammed Abouzour [Sybase] abouz...@gmail.com wrote: I am trying to upgrade our RB from 1.0rc3 to 1.5. We are running on top of a SQLAnywhere database server. I got the database upgraded successfully, but when I try to access the site it gives me errors regarding badly generated SQL queries. The problem is with the reviews_review.public field. This field is a Boolean field. The query that is generated by RB looks as follows: SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public AND ... In the SQLAnywhere Django driver, Boolean fields are mapped to BIT fields on the database side. As such, the correct query above should have been SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public = 1 AND ... I am inheriting this system so I am still learning both Django and RB. What component is responsible for generating the correct syntax for Boolean fields? -- Mohammed -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know athttp://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegroups.com reviewboard%2bunsubscr...@googlegroups.comreviewboard%252bunsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en
Re: Upgrading 1.0rc3 to 1.5 and Boolean fields queries
Yes, the SQLAnywhere database server driver is public (http:// code.google.com/p/sqlany-django/) but it will take some time before it becomes a standard part of the Django release. We will be actively maintaining and updating it. -- Mohammed On Oct 18, 5:57 pm, Christian Hammond chip...@chipx86.com wrote: Hi, To my knowledge, nobody is using Oracle. rb-site doesn't provide it as an option, and we don't have a license with which to test. Most people so far have been using MySQL and PostgreSQL. Is the Sybase driver becoming public, and will it be part of Django in the future? Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.reviewboard.org VMware, Inc. -http://www.vmware.com On Mon, Oct 18, 2010 at 1:49 PM, Mohammed Abouzour [Sybase] abouz...@gmail.com wrote: Hi Christian, Thanks, that did solve my problem ... but it made me wonder how other backend drivers, namely Oracle, handled this SQL without = 1 ? -- Mohammed On Oct 18, 3:31 pm, Christian Hammond chip...@chipx86.com wrote: Hi Mohammed, This query is generated in both reviews/managers.py and reviews/datagrids.py. In these cases, it's raw SQL, so you should be able to easily modify that. Preferably, we'd move away from raw SQL now that Django has support for F() expressions (things like field_name = field_name + 1 and stuff, which it couldn't do before). But a quick fix would be to just do the = 1 on those queries. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.reviewboard.org VMware, Inc. -http://www.vmware.com On Mon, Oct 18, 2010 at 8:01 AM, Mohammed Abouzour [Sybase] abouz...@gmail.com wrote: I am trying to upgrade our RB from 1.0rc3 to 1.5. We are running on top of a SQLAnywhere database server. I got the database upgraded successfully, but when I try to access the site it gives me errors regarding badly generated SQL queries. The problem is with the reviews_review.public field. This field is a Boolean field. The query that is generated by RB looks as follows: SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public AND ... In the SQLAnywhere Django driver, Boolean fields are mapped to BIT fields on the database side. As such, the correct query above should have been SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public = 1 AND ... I am inheriting this system so I am still learning both Django and RB. What component is responsible for generating the correct syntax for Boolean fields? -- Mohammed -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know athttp://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegroups.com reviewboard%2bunsubscr...@googlegroups.comreviewboard%252bunsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know athttp://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en
Re: Upgrading 1.0rc3 to 1.5 and Boolean fields queries
Awesome :) If you guys wanted to give us a patch to rb-site to support it, we'll include it (provided that you can easy_install the module). Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.reviewboard.org VMware, Inc. - http://www.vmware.com On Mon, Oct 18, 2010 at 4:46 PM, Mohammed Abouzour [Sybase] abouz...@gmail.com wrote: Yes, the SQLAnywhere database server driver is public (http:// code.google.com/p/sqlany-django/) but it will take some time before it becomes a standard part of the Django release. We will be actively maintaining and updating it. -- Mohammed On Oct 18, 5:57 pm, Christian Hammond chip...@chipx86.com wrote: Hi, To my knowledge, nobody is using Oracle. rb-site doesn't provide it as an option, and we don't have a license with which to test. Most people so far have been using MySQL and PostgreSQL. Is the Sybase driver becoming public, and will it be part of Django in the future? Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.reviewboard.org VMware, Inc. -http://www.vmware.com On Mon, Oct 18, 2010 at 1:49 PM, Mohammed Abouzour [Sybase] abouz...@gmail.com wrote: Hi Christian, Thanks, that did solve my problem ... but it made me wonder how other backend drivers, namely Oracle, handled this SQL without = 1 ? -- Mohammed On Oct 18, 3:31 pm, Christian Hammond chip...@chipx86.com wrote: Hi Mohammed, This query is generated in both reviews/managers.py and reviews/datagrids.py. In these cases, it's raw SQL, so you should be able to easily modify that. Preferably, we'd move away from raw SQL now that Django has support for F() expressions (things like field_name = field_name + 1 and stuff, which it couldn't do before). But a quick fix would be to just do the = 1 on those queries. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.reviewboard.org VMware, Inc. -http://www.vmware.com On Mon, Oct 18, 2010 at 8:01 AM, Mohammed Abouzour [Sybase] abouz...@gmail.com wrote: I am trying to upgrade our RB from 1.0rc3 to 1.5. We are running on top of a SQLAnywhere database server. I got the database upgraded successfully, but when I try to access the site it gives me errors regarding badly generated SQL queries. The problem is with the reviews_review.public field. This field is a Boolean field. The query that is generated by RB looks as follows: SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public AND ... In the SQLAnywhere Django driver, Boolean fields are mapped to BIT fields on the database side. As such, the correct query above should have been SELECT COUNT(*) FROM reviews_review, ... WHERE reviews_review.public = 1 AND ... I am inheriting this system so I am still learning both Django and RB. What component is responsible for generating the correct syntax for Boolean fields? -- Mohammed -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know athttp://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegroups.com reviewboard%2bunsubscr...@googlegroups.comreviewboard%252bunsubscr...@googlegroups.com reviewboard%2bunsubscr...@googlegroups.comreviewboard%252bunsubscr...@googlegroups.com reviewboard%252bunsubscr...@googlegroups.comreviewboard%25252bunsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know athttp://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegroups.com reviewboard%2bunsubscr...@googlegroups.comreviewboard%252bunsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/
Re: Issue 929 in reviewboard: reviewboard should not alter the diff file
Comment #4 on issue 929 by chipx86: reviewboard should not alter the diff file http://code.google.com/p/reviewboard/issues/detail?id=929 It does do this. It just breaks it up per-file (which is needed). The problem is that the parser doesn't know about some of the extra metadata. So really, the parser just needs to be fixed to not lose this. -- You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-iss...@googlegroups.com. To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en.
Re: Issue 921 in reviewboard: Add post-commit review submission to post-review for perforce
Updates: Summary: Add post-commit review submission to post-review for perforce Labels: -Type-Defect Type-Enhancement Comment #2 on issue 921 by trowbrds: Add post-commit review submission to post-review for perforce http://code.google.com/p/reviewboard/issues/detail?id=921 (No comment was entered for this change.) -- You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-iss...@googlegroups.com. To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en.
Re: Issue 929 in reviewboard: reviewboard should not alter the diff file
Comment #5 on issue 929 by d...@n-cube.org: reviewboard should not alter the diff file http://code.google.com/p/reviewboard/issues/detail?id=929 I wonder if this wouldn't still be a good idea. I was trying to review http://reviews.reviewboard.org/r/1737/ the other day because I thought it might address this problem, at least for git. However, besides missing the mail headers from format-patch, it was also missing the new file mode lines, so even once I had an appropriate base to apply on, I had to manually edit the diff just to get it to apply. I suspect there's always going to be the risk of losing portions of a diff if you break up and process it and then try to recreate it. Saving a pristine 'patch' alongside the reviewboard internal diff segments might actually be a more robust approach. Whatever we do, we really need to do something. I was hoping Eduardo's patch would do this, but it definitely had some bugs in it and I'm a bit confused over the 'bundle' concept he introduced since a format-patch output is only a single patch... -- You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-iss...@googlegroups.com. To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en.
