You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Julian Hyde <jh...@apache.org> on 2014/12/01 22:56:37 UTC
Commit comments
When committing changes that fix a JIRA case, please adhere to the exact
format
[CALCITE-n] Jira case description
for example
[CALCITE-370] Support GROUPING SETS, CUBE, ROLLUP in SQL and algebra
This will allow us to use scripts to process commit logs. A couple of
commits recently were close but not, in my opinion, close enough:
[CALCITE-488] - Enumerable<Holder> where Holder is custom class with a
single field does not work Calcite tries to treat it as SCALAR due
to "premature" JavaRowFormat.optimize (32 hours ago) <Vladimir Sitnikov>
CALCITE-352. Throw exception if ResultSet#next() is called after close() (2
days ago) <Siva Narayanan>
[CALCITE-465] Remove OneRow and Empty relational expressions; Values will
suffice (2 weeks ago) <Julian Hyde>
In the last one, note the space at the front of the line (mea culpa).
Committers, if the patch does not have a good description, feel free to fix
it (using ‘git rebase -i’ or otherwise) before you commit.
Writing a good JIRA case description is a different matter, but try to keep
it short, and describe the case in terms meaningful to the end-user. This
line will end up in the release notes.
If your understanding of the case changes as you work on it, change the
description. For example, https://issues.apache.org/jira/browse/CALCITE-403
was originally logged as "Enumerable gives NullPointerException with
HAVING on nullable GROUP BY column", then changed to "Enumerable gives
NullPointerException with HAVING on nullable expression" and was
"Enumerable gives NullPointerException with NOT on nullable expression" by
the time it was marked fixed.
Julian
Re: Commit comments
Posted by James Taylor <ja...@apache.org>.
Yes, we'd add the ‘(Siva Narayanan)’ in that case. Most of our
patches come in as patch files attached to the JIRA rather than pull
requests, so this makes it easier to tell from the git log who
authored the change. There is some redundancy, though, because we
typically set the assignee of the JIRA to the author too. We adopted
this based on feedback from the HBase committers that work on Phoenix
as well.
On Mon, Dec 1, 2014 at 3:14 PM, Julian Hyde <jh...@apache.org> wrote:
> We preserve the github author in the commit, if the change comes from
> a pull request. For example,
> https://github.com/apache/incubator-calcite/commit/59657ec75981fb1416d127c413a8dd7ae3571a1d
> was authored by Siva, committed by Vladimir. Would you add ‘ (Siva
> Narayanan)’ in this case?
>
> It’s not a bad idea to give contributors explicit credit. Just trying
> to find out the details of the policy.
>
> Julian
>
> On Mon, Dec 1, 2014 at 2:21 PM, James Taylor <ja...@apache.org> wrote:
>> We follow the same in Phoenix, with one more bit of information. If
>> committing a patch on behalf of someone else (who may not be a
>> committer), we add their name in parenthesis at the end of the commit
>> message like this:
>>
>> CALCITE-352. Throw exception if ResultSet#next() is called after
>> close() (John Doe)
>>
>> On Mon, Dec 1, 2014 at 1:56 PM, Julian Hyde <jh...@apache.org> wrote:
>>> When committing changes that fix a JIRA case, please adhere to the exact
>>> format
>>>
>>> [CALCITE-n] Jira case description
>>>
>>> for example
>>>
>>> [CALCITE-370] Support GROUPING SETS, CUBE, ROLLUP in SQL and algebra
>>>
>>> This will allow us to use scripts to process commit logs. A couple of
>>> commits recently were close but not, in my opinion, close enough:
>>>
>>> [CALCITE-488] - Enumerable<Holder> where Holder is custom class with a
>>> single field does not work Calcite tries to treat it as SCALAR due
>>> to "premature" JavaRowFormat.optimize (32 hours ago) <Vladimir Sitnikov>
>>> CALCITE-352. Throw exception if ResultSet#next() is called after close() (2
>>> days ago) <Siva Narayanan>
>>> [CALCITE-465] Remove OneRow and Empty relational expressions; Values will
>>> suffice (2 weeks ago) <Julian Hyde>
>>>
>>> In the last one, note the space at the front of the line (mea culpa).
>>>
>>> Committers, if the patch does not have a good description, feel free to fix
>>> it (using ‘git rebase -i’ or otherwise) before you commit.
>>>
>>> Writing a good JIRA case description is a different matter, but try to keep
>>> it short, and describe the case in terms meaningful to the end-user. This
>>> line will end up in the release notes.
>>>
>>> If your understanding of the case changes as you work on it, change the
>>> description. For example, https://issues.apache.org/jira/browse/CALCITE-403
>>> was originally logged as "Enumerable gives NullPointerException with
>>> HAVING on nullable GROUP BY column", then changed to "Enumerable gives
>>> NullPointerException with HAVING on nullable expression" and was
>>> "Enumerable gives NullPointerException with NOT on nullable expression" by
>>> the time it was marked fixed.
>>>
>>> Julian
Re: Commit comments
Posted by Julian Hyde <jh...@apache.org>.
We preserve the github author in the commit, if the change comes from
a pull request. For example,
https://github.com/apache/incubator-calcite/commit/59657ec75981fb1416d127c413a8dd7ae3571a1d
was authored by Siva, committed by Vladimir. Would you add ‘ (Siva
Narayanan)’ in this case?
It’s not a bad idea to give contributors explicit credit. Just trying
to find out the details of the policy.
Julian
On Mon, Dec 1, 2014 at 2:21 PM, James Taylor <ja...@apache.org> wrote:
> We follow the same in Phoenix, with one more bit of information. If
> committing a patch on behalf of someone else (who may not be a
> committer), we add their name in parenthesis at the end of the commit
> message like this:
>
> CALCITE-352. Throw exception if ResultSet#next() is called after
> close() (John Doe)
>
> On Mon, Dec 1, 2014 at 1:56 PM, Julian Hyde <jh...@apache.org> wrote:
>> When committing changes that fix a JIRA case, please adhere to the exact
>> format
>>
>> [CALCITE-n] Jira case description
>>
>> for example
>>
>> [CALCITE-370] Support GROUPING SETS, CUBE, ROLLUP in SQL and algebra
>>
>> This will allow us to use scripts to process commit logs. A couple of
>> commits recently were close but not, in my opinion, close enough:
>>
>> [CALCITE-488] - Enumerable<Holder> where Holder is custom class with a
>> single field does not work Calcite tries to treat it as SCALAR due
>> to "premature" JavaRowFormat.optimize (32 hours ago) <Vladimir Sitnikov>
>> CALCITE-352. Throw exception if ResultSet#next() is called after close() (2
>> days ago) <Siva Narayanan>
>> [CALCITE-465] Remove OneRow and Empty relational expressions; Values will
>> suffice (2 weeks ago) <Julian Hyde>
>>
>> In the last one, note the space at the front of the line (mea culpa).
>>
>> Committers, if the patch does not have a good description, feel free to fix
>> it (using ‘git rebase -i’ or otherwise) before you commit.
>>
>> Writing a good JIRA case description is a different matter, but try to keep
>> it short, and describe the case in terms meaningful to the end-user. This
>> line will end up in the release notes.
>>
>> If your understanding of the case changes as you work on it, change the
>> description. For example, https://issues.apache.org/jira/browse/CALCITE-403
>> was originally logged as "Enumerable gives NullPointerException with
>> HAVING on nullable GROUP BY column", then changed to "Enumerable gives
>> NullPointerException with HAVING on nullable expression" and was
>> "Enumerable gives NullPointerException with NOT on nullable expression" by
>> the time it was marked fixed.
>>
>> Julian
Re: Commit comments
Posted by James Taylor <ja...@apache.org>.
We follow the same in Phoenix, with one more bit of information. If
committing a patch on behalf of someone else (who may not be a
committer), we add their name in parenthesis at the end of the commit
message like this:
CALCITE-352. Throw exception if ResultSet#next() is called after
close() (John Doe)
On Mon, Dec 1, 2014 at 1:56 PM, Julian Hyde <jh...@apache.org> wrote:
> When committing changes that fix a JIRA case, please adhere to the exact
> format
>
> [CALCITE-n] Jira case description
>
> for example
>
> [CALCITE-370] Support GROUPING SETS, CUBE, ROLLUP in SQL and algebra
>
> This will allow us to use scripts to process commit logs. A couple of
> commits recently were close but not, in my opinion, close enough:
>
> [CALCITE-488] - Enumerable<Holder> where Holder is custom class with a
> single field does not work Calcite tries to treat it as SCALAR due
> to "premature" JavaRowFormat.optimize (32 hours ago) <Vladimir Sitnikov>
> CALCITE-352. Throw exception if ResultSet#next() is called after close() (2
> days ago) <Siva Narayanan>
> [CALCITE-465] Remove OneRow and Empty relational expressions; Values will
> suffice (2 weeks ago) <Julian Hyde>
>
> In the last one, note the space at the front of the line (mea culpa).
>
> Committers, if the patch does not have a good description, feel free to fix
> it (using ‘git rebase -i’ or otherwise) before you commit.
>
> Writing a good JIRA case description is a different matter, but try to keep
> it short, and describe the case in terms meaningful to the end-user. This
> line will end up in the release notes.
>
> If your understanding of the case changes as you work on it, change the
> description. For example, https://issues.apache.org/jira/browse/CALCITE-403
> was originally logged as "Enumerable gives NullPointerException with
> HAVING on nullable GROUP BY column", then changed to "Enumerable gives
> NullPointerException with HAVING on nullable expression" and was
> "Enumerable gives NullPointerException with NOT on nullable expression" by
> the time it was marked fixed.
>
> Julian