[GitHub] rob05c commented on a change in pull request #2102: Add TO all 1.2 CDNs routes, remove perl
rob05c commented on a change in pull request #2102: Add TO all 1.2 CDNs routes, remove perl URL: https://github.com/apache/incubator-trafficcontrol/pull/2102#discussion_r180471719 ## File path: traffic_ops/traffic_ops_golang/cdn/cdns.go ## @@ -368,3 +371,109 @@ func deleteQuery() string { WHERE id=:id` return query } + +func DeleteNameHandler(db *sql.DB) http.HandlerFunc { Review comment: @DylanVolz How opposed are you to this? So, I started trying to use the abstraction stuff, and it had a pretty steep learning curve. It looked like it would have to be nontrivially extended to support named deletes, too (Or maybe I just didn't understand it well enough). This was a lot easier to write, using helper funcs instead of abstraction, interfaces, reflection, etc. It'll be a lot easier to extend too, I think. With the helper funcs, it's 55 lines, versus 40 for the Delete func. We could probably make a few more helper funcs, too (maybe one to get all the commonly-needed objects, context, handleErrs, user, etc in one line). How would you feel about going forward with this style, with helper funcs instead of interfaces & reflection? I know it's a little more duplication, but IMO it'd be a lot easier for new people to get in and start working on the codebase. I think it'd be easier for us to extend when necessary too. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rob05c commented on a change in pull request #2102: Add TO all 1.2 CDNs routes, remove perl
rob05c commented on a change in pull request #2102: Add TO all 1.2 CDNs routes, remove perl URL: https://github.com/apache/incubator-trafficcontrol/pull/2102#discussion_r180471719 ## File path: traffic_ops/traffic_ops_golang/cdn/cdns.go ## @@ -368,3 +371,109 @@ func deleteQuery() string { WHERE id=:id` return query } + +func DeleteNameHandler(db *sql.DB) http.HandlerFunc { Review comment: @DylanVolz How opposed are you to this? So, I started trying to use the abstraction stuff, and it had a pretty steep learning curve. It looked like it would have to be nontrivially extended to support named deletes, too (Or maybe I just didn't understand it well enough). This was a lot easier to write, using helper funcs instead of abstraction, interfaces, reflection, etc. It'll be a lot easier to extend too, I think. With the helper funcs, it's 55 lines, versus 40 for the Delete func. We could probably make a few more helper funcs, too (maybe one to get all the commonly-needed objects, context, handleErrs, user, etc in one line). How would you feel about going forward with this style, with helper funcs instead of interfaces & reflection? I know it's a little more duplication, but IMO it'd be a lot easier for new people to get in and start working on the codebase. I think it'd be easier for us to extend when necessary too. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services