djnavarro commented on pull request #12154: URL: https://github.com/apache/arrow/pull/12154#issuecomment-1014916566
My thinking at this point is that (if this is consistent with the general etiquette here!) I'd like to (1) work out why the build is failing two checks and fix those, (2) merge the passing version, (3) open tickets for the missing functionality, one for the C++ side and one for the R side (and assign myself to the R one 😁 ). My reasons for suggesting this are: - The current version does work for most use cases. The `week_start` issue only applies when the rounding unit is "week". As it stands we can round to "7 days" or indeed any time unit except "week" and have it mirror lubridate almost exactly. I've modified the PR so that we abandon ship if the user specifies "week" as the rounding unit, and falls back on lubridate. - It gives me an excuse to do a deeper dive on timezones before merging the current version: if I'm going to write more lubridate bindings later this is something I'm going to need to know anyway! - Once it's merged it would be clearer what modifications are needed to supply the missing functionality, so I can write the JIRA tickets a bit more precisely - A little selfishly... I presume that once I am no longer a first-time contributor I won't need to constantly bug maintainers to enable the GH actions workflows - Slightly less selfishly, it would be mildly convenient for writing blog content about the process. I think there's value in highlighting that a PR can still be useful when surfaces issues pertaining to missing functionality and in doing so immediately triggers new issues? Possible reasons not to: - As flagged above I'm not sure of the etiquette here. If it's not considered appropriate I'm totally happy to leave this hanging! - The `change_on_boundary` issue does mean that the Arrow binding for `ceiling_date()` introduces a minor discrepancy. We're currently mirroring a slightly older lubridate default. I'm not sure that's currently a big issue, but it's something we'd definitely want to revisit before this appears in a release I'm happy to follow advice here. I'm totally find with whatever approach more experienced folks think is best! 🙂 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
