The template says "including all aspects such as the documentation fix 
<https://github.com/apache/spark/blob/ffb31565e5af6f9ab2f8f7b500fbd595c8bb5a58/.github/PULL_REQUEST_TEMPLATE#L34-L36>.”
 The original intent may well have been about behavior changes only, but that’s 
not reflected in the current text of the PR template.


> On Jan 16, 2025, at 2:32 PM, Dongjoon Hyun <dongjoon.h...@gmail.com> wrote:
> 
> The original intent is a user-facing *behavior* change technically
> which is the same with Apache Spark migration guide.
> 
> If so, does it make sense to you?
> 
> Probably, since the template was short to be concise, it could be interpreted 
> in more ways than we thought.
> 
> Dongjoon.
> 
> On Thu, Jan 16, 2025 at 11:16 AM Nicholas Chammas <nicholas.cham...@gmail.com 
> <mailto:nicholas.cham...@gmail.com>> wrote:
>> I didn’t write the pull request template and I am not sure about the 
>> author’s original intent.
>> 
>> However, the plain meaning of “any” — with the added emphasis 
>> <https://github.com/apache/spark/blob/ffb31565e5af6f9ab2f8f7b500fbd595c8bb5a58/.github/PULL_REQUEST_TEMPLATE#L34-L36>
>>  as well — suggests that the author of this policy did mean that yes, even a 
>> typo fix in a user-facing documentation page merits a “yes” response to this 
>> question.
>> 
>> It’s strict but it’s also clear and unambiguous. No one has to think about 
>> whether their user-facing change is big enough to merit a yes.
>> 
>> IMO this is better than a more ambiguous policy that says something like 
>> “yes if it’s user-facing _and_ is not a trivial fix”, because then there 
>> will be disagreements about what is trivial vs. not.
>> 
>> But my opinion on the correct policy doesn’t matter that much since I’m not 
>> a committer and not a heavy reviewer. I mostly wanted to raise this so that 
>> we could somehow resolve this apparent disconnect between the plain meaning 
>> of the PR template and how PR authors are responding to it.
>> 
>> 
>>> On Jan 16, 2025, at 2:02 PM, Dongjoon Hyun <dongjoon.h...@gmail.com 
>>> <mailto:dongjoon.h...@gmail.com>> wrote:
>>> 
>>> I understand your concern, Nicholas. However, isn't it too strict?
>>> 
>>> For the above example, adding a new HTML page is a user-facing change.
>>> 
>>> https://github.com/apache/spark/pull/48852 (This is a new doc)
>>> [SPARK-50309][DOCS] Document SQL Pipe Syntax
>>> 
>>> https://github.com/apache/spark/pull/49098 (This is one line link addition)
>>> [SPARK-48426][DOCS][FOLLOWUP] Add `Operators` page to `sql-ref.md`
>>> 
>>> Given your definition, even a word typo fix inside an HTML page becomes a 
>>> user-facing change.
>>> Did I understand your definition correctly? Or, is it something else?
>>> 
>>> Dongjoon.
>>> 
>>> 
>>> On Thu, Jan 16, 2025 at 9:15 AM Nicholas Chammas 
>>> <nicholas.cham...@gmail.com <mailto:nicholas.cham...@gmail.com>> wrote:
>>>> This is not a big deal at all, but I figure it’s worth bringing up briefly 
>>>> because the pull request template does emphasize 
>>>> <https://github.com/apache/spark/blob/ffb31565e5af6f9ab2f8f7b500fbd595c8bb5a58/.github/PULL_REQUEST_TEMPLATE#L34-L36>:
>>>> 
>>>> > ### Does this PR introduce _any_ user-facing change?
>>>> > 
>>>> > Note that it means *any* user-facing change including all aspects such 
>>>> > as the documentation fix.
>>>> 
>>>> And yet I regularly see PRs where the author states “no user-facing 
>>>> change” even though there are clearly user-facing documentation changes 
>>>> included in the diff.
>>>> 
>>>> Some examples:
>>>> - https://github.com/apache/spark/pull/47756
>>>> - https://github.com/apache/spark/pull/49098
>>>> - https://github.com/apache/spark/pull/48852
>>>> - https://github.com/apache/spark/pull/49273
>>>> 
>>>> Again, not a big deal. But I assume committers care about this since it’s 
>>>> in the PR template.
>>>> 
>>>> We should remind contributors (and each other) to answer this question 
>>>> correctly when appropriate, or we should perhaps update the PR template if 
>>>> the guidance there is outdated.
>>>> 
>>>> Nick
>>>> 
>> 

Reply via email to