Seems like we should fix the template if that's not the intent .... On Thu, Jan 16, 2025 at 1:52 PM Nicholas Chammas <nicholas.cham...@gmail.com> wrote:
> 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> 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 >>> >>> >> >
