← Back to team overview

graphite-dev team mailing list archive

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