| opendevreview | suzhengwei proposed openstack/devstack master: Glance: add version to service_url if uwsgi mode https://review.opendev.org/c/openstack/devstack/+/907511 | 01:46 |
|---|---|---|
| *** dtantsur_ is now known as dtantsur | 01:50 | |
| opendevreview | Ashley Rodriguez proposed openstack/devstack-plugin-ceph master: Add ingress deamon https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/900348 | 13:24 |
| dansmith | clarkb: a while back I think you filed a bug about making the "file size that disabled syntax highlighting" configurable with gerrit... anything ever come of that? | 14:53 |
| dansmith | it's brutal reviewing anything in nova without syntax highlighting | 14:53 |
| opendevreview | Ashley Rodriguez proposed openstack/devstack-plugin-ceph master: Bump to Reef https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/907614 | 15:37 |
| opendevreview | Ashley Rodriguez proposed openstack/devstack-plugin-ceph master: Add ingress deamon https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/900348 | 15:37 |
| clarkb | I'm not sure I filed a bug, I dug up the limits in the code so that people were aware of why it was happening. I'll check in the bug tracker to see if I filed anything | 16:24 |
| clarkb | dansmith: https://issues.gerritcodereview.com/issues/40015291 this appears to be what I filed. It hasn't gotten any attention | 16:25 |
| dansmith | ugh | 16:25 |
| dansmith | is it unreasonable to ask if we could tweak the limits in our code as deployed? | 16:25 |
| clarkb | now that we are no longer running a forked gerrit we're trying hard not to fall back on that | 16:28 |
| clarkb | it led to major issues over the yaers which eventually led to us being many years behind in the update cycle which was difficult to dig out of | 16:29 |
| clarkb | compared to now where we regularly run the latest gerrit release for portions of the year | 16:29 |
| dansmith | seems like a patch in the workflow to change some constants is probably not a huge deal, but okay | 16:30 |
| dansmith | it's just incredibly painful | 16:30 |
| clarkb | 3.9 has some diff size limit. I almost wonder if we can convince them to add a syntax highlight limit alongside that | 16:32 |
| clarkb | since the new item is there to address similar client performance concerns | 16:32 |
| dansmith | so the current size limit is 500kb yeah? Lemme see how many files we have over that limit | 16:35 |
| clarkb | let me double check the current version of gerrit | 16:35 |
| clarkb | export const CODE_MAX_LINES = 20 * 1000; const CODE_MAX_LENGTH = 25 * CODE_MAX_LINES; | 16:37 |
| clarkb | seems to be the same limit so ya 500kb roughly | 16:37 |
| clarkb | it checks length which could be character length and not byte length but for most of our files these numbers are probably the same | 16:38 |
| clarkb | I asked in their discord server if we can make these limits configurable and/or provide the warning as requested in that issue | 16:40 |
| clarkb | let me see if gerrit has any files this large that we can use as a reproduction case for upstream | 16:43 |
| clarkb | only the packfile which isn't part of the diffs users see | 16:44 |
| clarkb | and I guess one png | 16:47 |
| clarkb | nova has 5 python files that large | 16:48 |
| clarkb | only one of nova's files is significantly larger than 500kb. I wonder if google did a source code size scan against all their code to decide on that limit | 16:49 |
| frickler | clarkb: did you check file size or # of lines? IMHO a python file with 20k lines is much too large for reviewing or anything anyway | 16:55 |
| clarkb | frickler: filesize using `find ./ -type f -size +500000c` | 16:58 |
| clarkb | the line count is a little less of a problem because it give you a little popup telling you why there is no highlighting | 16:58 |
| clarkb | but in the filesize case it does not and is extra confusing | 16:58 |
| clarkb | this is one downside to using spaces for indentation I guess. Lots of extra bytes | 17:03 |
| dansmith | clarkb: sorry, call.. yeah the files that are over that size are the hottest areas of nova work of coruse | 17:32 |
| clarkb | dansmith: ya for the test files as an outsider looking in it seems like those could be trimmed relatively easily. More difficult for the core bits of nova | 17:36 |
| dansmith | they can all be split of course, but there's really no compelling reason to do it (other than this) because reviews to those files are usually just test case additions | 17:37 |
| dansmith | the core bit can as well, but again, desktop editors have no problem with them and being able to fold the whole file into classes and methods makes finding things much easier than if they're split | 17:37 |
| dansmith | if it was 1992 and we all have 4mb of ram, it'd be different, but... :) | 17:37 |
| *** priteau_ is now known as priteau | 21:44 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!