Sorry Vlad - I disagree. Where is the simple fix? As a new contributor, you should not be coming in guns blazing blaming committers who are trying to keep the master branch sane and clean.
On Tue, Mar 25, 2025 at 10:53 PM Rozov, Vlad <vro...@amazon.com.invalid> wrote: > There is a simple fix. This is exactly what I outlined in the e-mail. > Prior to reverting commit (on master) it was necessary to see if an easy > fix exists. The PR that introduced the error was merged into master 3 > weeks ago, so I still don’t get why it was reverted overnight. It was > also necessary to open JIRA and provide reproducible steps. I assume that > steps on the PR is what needs to be fixed, but it will be better to avoid > guessing. > > Thank you, > > Vlad > > On Mar 25, 2025, at 10:05 PM, Reynold Xin <r...@databricks.com.INVALID> > wrote: > > Is there a fix already available or a very simple fix a committer can > create quickly? If yes, we can merge the fix. If there isn't, for major > functionality breaking change, we should just revert. That's fairly basic > software engineering practices. > > > On Tue, Mar 25, 2025 at 9:53 PM Hyukjin Kwon <gurwls...@apache.org> wrote: > >> With the change, the main entry points, Spark shalls, don't work and >> developers cannot debug and test. The snapshots become uesless. >> >> The tests passed because you did not fix SBT. It needs a larger change. >> >> Such change cannot be in the source. I can start a vote if you think this >> is an issue. >> >> >> On Wed, Mar 26, 2025 at 12:32 PM Rozov, Vlad <vro...@amazon.com.invalid> >> wrote: >> >>> This does not make any sense. >>> >>> 1. There are no broken tests introduced by >>> https://github.com/apache/spark/pull/49971 >>> 2. There are no JIRA filed for “the main entry point” >>> 3. “The main entry point” that does not have any unit test suggests that >>> it is not the main entry point. >>> 4. It is not practical to revert every commit that introduced some >>> functional issue or regression. If regression can’t be fixed, it is >>> reverted, if it can be reasonably fixed, the fix is applied to the >>> regression. Just as an example, >>> https://github.com/apache/spark/pull/42823 introduced a regression. It >>> was fixed by https://github.com/apache/spark/pull/50348. Nobody asked >>> for the revert. >>> 5. The way how revert was handled is not "Apache way”. >>> >>> It may be easier for you to handle it in a single PR, but it is easier >>> for me to provide PR on top of existing changes and it will also provide >>> JIRA and a commit that documents why there is an issue. Otherwise it will >>> be easier to introduce the error back again. >>> >>> Thank you, >>> >>> Vlad >>> >>> >>> On Mar 25, 2025, at 4:58 PM, Hyukjin Kwon <gurwls...@apache.org> wrote: >>> >>> Rozov, this broke the main entry points of release, Spark shells. Even >>> in the mast branch, you build a Spark, and cannot use Spark shells. >>> Why don't you submit a PR that contains the proper fix? It is easier to >>> have one PR that has no issue, e.g., reverting backporting etc. >>> >>> On Wed, 26 Mar 2025 at 00:17, Rozov, Vlad <vro...@amazon.com.invalid> >>> wrote: >>> >>>> Hi All, >>>> >>>> I kind of understand why https://github.com/apache/spark/pull/49971 was >>>> reverted on the branch-4.0 to allow testing of 4.0 release. Why was it also >>>> reverted on the master branch? I don’t see any JIRA being open for the >>>> failure. AFAIK, the proper way to handle the issue in Apache project: >>>> >>>> - revert https://github.com/apache/spark/pull/49971 on the branch-4.0 >>>> (note that PR targeted master branch and SPARK-51229 targets 4.1 as >>>> affected version) >>>> - open JIRA and provide details of how to reproduce the error on the >>>> master branch >>>> - wait for the author of https://github.com/apache/spark/pull/49971 (in >>>> this case it was me) to reply in a timely manner and only then revert the >>>> commit >>>> >>>> If you agree with the approach, please undo the revert and file JIRA. >>>> There is an easy fix to the issue. >>>> >>>> Thank you, >>>> >>>> Vlad >>>> >>>> >>>> >>> >
