-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
address #2466, incorporate #2468 and #2469 #2470
Conversation
// PR 2468 | ||
// if (gt == wkbMultiSurface || gt == wkbMultiPolygon) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't bring much, right ?
// PR 2468 | |
// if (gt == wkbMultiSurface || gt == wkbMultiPolygon) { |
// PR 2469 | ||
/* OGRGeometryCollection *a = (OGRGeometryCollection *) g[i]; | ||
out[i] = a->get_Length();*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// PR 2469 | |
/* OGRGeometryCollection *a = (OGRGeometryCollection *) g[i]; | |
out[i] = a->get_Length();*/ |
@rouault I leave commented-out code that may be discarded to make off-github review clearer. It can be dropped later. This is @edzer 's codebase, so I don't delete on my fork and PRs, I let him do that if he agrees. Since I (still) prefer to run all the R checks locally first (because I have control of GDAL etc., versions), I am conservative in letting github work things out. I edit in vim, as you might expect. |
I don't get it: that's git (the software and code repository format) which keeps the old versions. Not github which is just a UI over it. So there's no harm in deleting unused code... It can always been recovered through git revert, etc. That's I believe the good practice. At least in the projects I'm directly involved in ;-) |
Certainly good practice. I started with SCCS in 1985. Here, @edzer is lead and makes the calls, I chip in (not often, and not often right), and work in my fork. So commenting out replaced code shows the diffs "in place" for visual review. I'm unlikely to become more confident in my commits over time, I think. |
Thanks, both, for resolving this! |
Adding @rouault 's correction from gdal-devel, now updated with #2468 and #2469. As reported in #2466 the failing packages
stcos
andplantTracking
now pass with rc1 and rc2.