villebro commented on PR #31765: URL: https://github.com/apache/superset/pull/31765#issuecomment-2581011022
@gerbermichi thanks for this PR! A few questions/requests: 1. The PR description is very light on details for all the changes in the PR. Would it be possible to cover all changes in the description to provide more context to reviewers? 2. I see there's a change for both i18n and time grain formatting in this PR - would it be possible to split the PR into two different PRs? 3. I know testing i18n related features can be challenging, but would it be possible to add unit tests for the time grain formatting? Adding a test that failed before but passes after the changes tends to be a great way of showing what the current issue is, how the added/changed logic addresses that, and finally protect against future regressions. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
