Hi Attila, Thank you for reviewing this patch so quickly! I have talked to Anna, she is going to review it very soon and post on ReviewBoard.
Yes, I wanted to push this as one commit to trunk using the patch file, the small commits were only for reviewing. Szabolcs On Tue, Jan 9, 2018 at 3:46 PM, Attila Szabó <mau...@apache.org> wrote: > Hey Szabi, > > Souds great! > > Two comments: > AFAIR Anna Szonyi was also planning to do something similar last January > (when she was busy around the build system, tests, gradle, etc.). It would > make sense on my side, to reach out to her, maybe she's got some useful > feedbacks for you too. > I've opened an issue on the review board (which if I'm not mistaken you've > already fixed meanwhile I've been preparing this email ;-) ). > > I'll give my +1 and ship it, as soon as I will be able to execute the > test/releasenotes/etc. on my side as well! > > Many thanks for jumping on this initiative, it's a quite old item on my > wishlist! > > Cheers, > Attila, > > ps: I do not have any pros or cons on the side of Pull Request VS. Review > Board, and I totally can understand if it's better for someone to review > 182 commits instead of a big one. Although I would strongly advise and > appreciate if the commits would be squashed into one before merging them to > the trunk, b/c testing the effects of 180+ commits, one by one would > consume tons of efforts. If the commit to the master would/will be up to > me, I would use the patch file instead, or squash first, just for the clean > state of the trunk as well. > > My2cents > > On Tue, Jan 9, 2018 at 11:49 AM, Ferenc Szabo <f...@cloudera.com> wrote: > > > Hi Szabi, > > > > I believe this is a great idea. > > > > By removing these packages we will get rid of a great deal of technical > > debt that will simplify future change. I will also help to avoid > > unnecessary conversions like the ones I had to use in my recent > > SqoopOptions related change. > > > > So, also +1 from me! > > > > Cheers, > > Fero > > > > On Mon, Jan 8, 2018 at 5:59 PM, Boglarka Egyed <b...@cloudera.com> > wrote: > > > > > Hi Szabolcs, > > > > > > I really welcome this initiative, it would be a huge clean up on this > > > project! > > > > > > I already took a look at your pull request and it indeed looks pretty > > > straightforward. > > > > > > I will perform a deeper review and publish it on the Review Board > > otherwise > > > +1 from my side for the idea in general, 1.5 release would be a good > > target > > > for this. > > > > > > Thanks for taking this effort! > > > > > > Cheers. > > > Bogi > > > > > > On Mon, Jan 8, 2018 at 2:01 PM, Szabolcs Vasas <va...@apache.org> > wrote: > > > > > > > Hi All, > > > > > > > > As you probably know we still have dozens of classes in > > > com.cloudera.sqoop > > > > packages which most of the cases just extend their corresponding > class > > in > > > > org.apache.sqoop package without adding extra functionality. > > > > These classes make the code harder to read and navigate, they are > > already > > > > deprecated but because of backward compatibility considerations we > were > > > not > > > > able to remove them. > > > > The community was planning on a release containing breaking changes > (we > > > > called it 1.5) I think it would be a great candidate to include this > > > > cleanup. > > > > I have created a ticket <https://issues.apache.org/ > > > jira/browse/SQOOP-3273> > > > > for doing this task and submitted a patch as well. I think the > easiest > > > way > > > > to take a look at it is to review the commits in my Sqoop fork: > > > > https://github.com/szvasas/sqoop/tree/cloudera_package_removal > > > > Note that this is change which basically affects all of the files in > > the > > > > Sqoop project, but in the majority of the cases replacing the > > > > com.cloudera.sqoop class to its corresponding org.apache.sqoop class > > was > > > > just a find and replace command so I consider this a relatively low > > risk > > > > change. > > > > Please feel free to reply to this email with your questions and > > concerns > > > > and if you have some time please take a look at the changes. > > > > > > > > Thanks and regards, > > > > Szabolcs > > > > > > > > > > > > > > > -- > > Ferenc Szabo > > Software Engineer > > > -- Szabolcs Vasas Software Engineer <http://www.cloudera.com>
