On Sat, Jun 23, 2018 at 10:05 AM Robert Butts <r...@apache.org> wrote:
> >as long as we have 100% feature complete with the Go code (which I think > we're fairly close?) > We are not close. We are at 108 of 240 endpoints. > > >if we do not treat the tests as first class citizens (meaning we implement > an Go endpoint and build coverage as we go), then why bother at all? > It doesn't have to be all or nothing. There's a happy medium between > requiring tests for every line of code, and having no tests. > > >I also thought that we were supporting removing the Perl in the next TC > 3.0 major release (several emails back). > That vote was to remove the Perl GUI, not all of Perl. > > > >Removing dead Perl code can be tricky, since the interdependencies are > not always obvious, and can lead to unforeseen missing functionality. > > That's a good point. But let me ask this: in the event we miss some dynamic > Perl calling a function that was removed, which is worse: to make that > function fail, or to make that function call dead code, which has been > changed and modified to behave differently in Go, and the undead Perl is > now doing the wrong thing? > In the case where a function is removed that overrode some default functionality (which happens a lot in this code), the default code would not fail, but produce different results. Not quite helpful.. However, the idea you present below mitigates that risk.. > So, there might be a middle ground. How about this: what if we remove the > Perl routes from the routes file, and change the functions to immediately > return an informative error message? That way, if we miss some dynamic Perl > still calling the code, we immediately get a useful message, so it's easy > to find and fix. And we don't call old, wrong code that could do something > dangerous and bad. And additionally, contributors in the community won't > see the functions that look real, and accidentally waste their time > modifying them. > This, I can live with, and I think it represents a good way forward: keep the signature of all functions -- do not remove any modules and replace the guts of each of those functions with a sensible error -- even with a stack trace so we can tell exactly what code path was used to get there. It should be fairly straightforward to create a utility function that can be used throughout so it happens consistently. > Those who voted against removing the dead code, does that sound like a > compromise you could live with? That's a yes from me. Thanks for coming up with a solid compromise.. -Dan > On Thu, Jun 21, 2018 at 7:52 AM, Dewayne Richardson <dewr...@apache.org> > wrote: > > > I'm +100 on removing dead Perl code as long as we have 100% feature > > complete with the Go code (which I think we're fairly close?). Rawlin > you > > are correct in that there is a spider web of interdependencies in Perl, > but > > without feature parity removing any of that dead code is like pulling a > > single thread from a sweater without unraveling the entire thing. In > other > > words let's put our efforts around Go features and moving forward. > > > > As for tests, I realize writing tests "takes time", but we have to build > a > > safety net for the Traffic Ops Go API's going forward. I also realize > that > > writing and maintaining the API tests is like maintaining two code bases, > > but if we do not treat the tests as first class citizens (meaning we > > implement an Go endpoint and build coverage as we go), then why bother at > > all? I'm pretty sure there is support for more testing and I also think > it > > has to be a "community" effort. > > > > I also thought that we were supporting removing the Perl in the next TC > 3.0 > > major release (several emails back). > > > > -Dew > > > > On Wed, Jun 20, 2018 at 9:54 AM Rawlin Peters <rawlin.pet...@gmail.com> > > wrote: > > > > > Hey Traffic Controllers, > > > > > > In order to accelerate our progress toward using and developing > > > traffic_ops_golang as a community, I'd like to propose that we remove > > > all of the dead Perl Traffic Ops API code in the repo. Many endpoints > > > have been rewritten in Go at this point, and by keeping the obsolete > > > Perl endpoints in the repo we're not making it clear that new > > > enhancements have to be made in traffic_ops_golang and Traffic Portal > > > in order to actually work and survive long-term (as opposed to the > > > legacy Perl Traffic Ops API and UI which is planned to be deprecated > > > and removed in the near future). > > > > > > Right now the only thing keeping the dead Perl endpoints from being > > > deleted is the Perl test framework that depends on them. Unfortunately > > > the tests are all caught in a spider web of interdependency, so we > > > can't simply remove tests for APIs that have been rewritten in Go > > > without breaking other tests. However, we think there are a few ways > > > we should be able to accomplish this: > > > > > > 1. Make the Perl integration tests actually make HTTP calls which can > > > then be handled by traffic_ops_golang, rather than calling the Perl > > > API router directly. > > > 2. Remove test interdependencies by mocking out the test data. > > > 3. Rewrite all the Perl tests in the go API test framework and remove > > > the Perl tests. > > > > > > We are also open to other suggestions that allow us to remove dead Perl > > > code :). > > > > > > Please +1 or -1 if you agree or disagree; all feedback is welcome. > > > > > > - Rawlin > > > > > >
