graphite-dev team mailing list archive
-
graphite-dev team
-
Mailing list archive
-
Message #00799
Re: [Merge] lp:~jdugan-x1024/graphite/esnet-tweaks into lp:graphite
Hey Jon, thanks for the branch work. 2-4 look good but I must admit to being a curmudgeon on #1. Here are my thoughts.
I did previously merge in the relative URL referencing stuff, and deployed it to my production environment. I ran into several issues over the course of a few weeks, various things ranging from a broken Ajax call to infinitely nested frames on the main page, to login issues, etc. All in all, I took the lazy route of simply reverting the changes. I'm sure it was probably a few small issues, but I wasn't feeling up to tracking them all down at the time.
The main issue I think is that, as you point out, it is a fairly invasive change. A minor change at many locations in the code. I think this makes it hard to get 100% right because it requires lots of testing. I'm not much of a tester personally, so I'd ask that if you want to merge this that you give it a thorough testing of all the affected code paths. I know that's a lot, but I think it is important for this type of change.
--
https://code.launchpad.net/~jdugan-x1024/graphite/esnet-tweaks/+merge/57403
Your team graphite-dev is requested to review the proposed merge of lp:~jdugan-x1024/graphite/esnet-tweaks into lp:graphite.
References