I also notice that this particular change (https://github.com/apache/calcite/commit/938614cca8c30ed9ff48996ff0ae42e1ed4f1706) was committed without going through code review. Do we have any process in place for merging a change? With the size and complexity of Calcite project, I would expect at least one +1 from other committers before merging the PR. Issues with the commit message would have been raised up if there was a proper code review.
> On Feb 21, 2020, at 9:47 AM, Julian Hyde <[email protected]> wrote: > > Committers, please remember that every time you accept a change, you > are writing the release notes. The commit messages will end up here: > https://calcite.apache.org/docs/history.html > > So, those commit messages need to make sense to end users. > > This is important because, as a project, we write very little > documentation. The only guide to what features are being added to the > project (other than reading the code) is those release notes. So, > those release notes need to be perfect. > > Case in point, this change: > https://github.com/apache/calcite/commit/938614cca8c30ed9ff48996ff0ae42e1ed4f1706, > with message "Support hint option key as string literal" > > As a SQL user I might happen to know that identifiers are unquoted or > enclosed in double quotes, and a string literal is always enclosed in > single quotes. Does this change mean that hint keys must now always be > enclosed in single quotes? I don't know. > > Maybe the message should be "Allow hint keys to be quoted". And > include an example in the JIRA case. > > I've said several times that if a change can be described in terms of > SQL, describe it in terms of SQL. Not in terms of java classes. (Yes, > some changes are changes to non-SQL APIs, or are entirely internal, or > are refactorings. Different rules apply.) > > And if you use SQL terms, capitalize them. Here's a good example: > "GROUP_ID returns wrong result". > > Julian
