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> 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>
> 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> 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