> On Oct. 31, 2014, 6:57 a.m., Jarek Cecho wrote: > > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java, lines > > 508-520 > > <https://reviews.apache.org/r/27330/diff/7/?file=743062#file743062line508> > > > > This move seems unencessary? > > Veena Basavaraj wrote: > why? would you care to ask why I felt it is necessary? > > when reading code, having all the links related apis in one places makes > it wasy for some one to not miss the apis. Whats wrong with moving, you want > another RB for it? say that. > > Jus think about how you organize your table, when we write code lets do > the same, organize things so that at one shot we know what the apis for links > are. I would even want to split this up into logical classes as we have groen > the repository apis, see how hard it is to add post gres, wish we had thight > through this before. > > Jarek Cecho wrote: > Doing unnecessary moves is breaking "git blame" and "git cherrypick". For > easier readability we have all the methods and properties named in a way that > most generic part was at the begging and most specific one at the end. This > way all the overview tools (like method listing in IDEs) is showing similar > methods next to each other. > > Veena Basavaraj wrote: > I just gave an example. > > we call it updateLink, and not linkUpdate, what is the most generic par? > > Annd if your concern is about git blame, I will do another RB first with > these changes. I have no issues with it.
Lets try to name methods like 90% of the people in other projects name it, if we have these special rules in sqoop, I;d like to see it documented and then we take a vote on why we need this. Until then I suggest we go back to what every one else names their methods by. This is something we should start right now helping contributors to be not confused on how to name a method and have a coherent story. First thing after this RB is shipped I will start the coding standard wiki and cover these examples - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27330/#review59331 ----------------------------------------------------------- On Oct. 30, 2014, 2:08 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27330/ > ----------------------------------------------------------- > > (Updated Oct. 30, 2014, 2:08 p.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see JIRA and its parent JIRA for details > > All the WS stuff, I will do it in the end once the functionality is reviewd. > > > If someone is wondering why START is changed to submit: there are tons of > places in the code and in the java docs, we actually mean submit when we say > START > > DRIVER_0008("Invalid combination of submission and execution engines"), > > DRIVER_0009("Job has been disabled. Cannot submit this job."), > > DRIVER_0010("Link for this job has been disabled. Cannot submit this job."), > > DRIVER_0011("Connector does not support specified direction. Cannot submit > this job."), > > ; > > > Diffs > ----- > > client/src/main/java/org/apache/sqoop/client/SqoopClient.java 33a0c3c > client/src/main/java/org/apache/sqoop/client/SubmissionCallback.java > de7211a > > client/src/main/java/org/apache/sqoop/client/request/JobResourceRequest.java > 83c08b3 > > client/src/main/java/org/apache/sqoop/client/request/SqoopResourceRequests.java > 4a56bb7 > > client/src/main/java/org/apache/sqoop/client/request/SubmissionResourceRequest.java > 5055783 > common/src/main/java/org/apache/sqoop/json/JobBean.java 082d591 > common/src/main/java/org/apache/sqoop/json/JobsBean.java PRE-CREATION > common/src/main/java/org/apache/sqoop/json/JsonBean.java ba86511 > common/src/main/java/org/apache/sqoop/json/LinksBean.java 5858a18 > common/src/main/java/org/apache/sqoop/json/SubmissionBean.java 4b80338 > common/src/main/java/org/apache/sqoop/json/SubmissionsBean.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MLink.java 7a9f538 > common/src/main/java/org/apache/sqoop/model/MSubmission.java 7290df5 > common/src/test/java/org/apache/sqoop/json/TestJobBean.java 1fc8dbd > common/src/test/java/org/apache/sqoop/json/TestLinkBean.java 811cbf0 > common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java e4d50bf > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcPartitioner.java > 2411169 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestPartitioner.java > 3ae64f0 > core/src/main/java/org/apache/sqoop/driver/JobManager.java f6447c6 > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 2aeb07e > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java > ad380d3 > core/src/main/java/org/apache/sqoop/repository/Repository.java 79742b9 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > b996a0b > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java > c894d06 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java > 4c2d062 > server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java > 5694ea5 > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > 5547988 > server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java > 35a9635 > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > 8555b0c > server/src/main/java/org/apache/sqoop/server/RequestHandler.java 508edd2 > server/src/main/java/org/apache/sqoop/server/v1/JobServlet.java d295237 > server/src/main/java/org/apache/sqoop/server/v1/JobsServlet.java > PRE-CREATION > server/src/main/java/org/apache/sqoop/server/v1/LinkServlet.java 127903a > server/src/main/java/org/apache/sqoop/server/v1/SubmissionServlet.java > 5c1d883 > server/src/main/java/org/apache/sqoop/server/v1/SubmissionsServlet.java > PRE-CREATION > server/src/main/webapp/WEB-INF/web.xml 6ad90d2 > shell/src/main/java/org/apache/sqoop/shell/AbortCommand.java PRE-CREATION > shell/src/main/java/org/apache/sqoop/shell/ShowJobStatusFunction.java > PRE-CREATION > shell/src/main/java/org/apache/sqoop/shell/SqoopShell.java 2e87965 > shell/src/main/java/org/apache/sqoop/shell/StartCommand.java 7c56980 > shell/src/main/java/org/apache/sqoop/shell/StartJobFunction.java 4363f05 > shell/src/main/java/org/apache/sqoop/shell/StatusCommand.java 3447a87 > shell/src/main/java/org/apache/sqoop/shell/StatusJobFunction.java fb83af3 > shell/src/main/java/org/apache/sqoop/shell/StopCommand.java 50b2e81 > shell/src/main/java/org/apache/sqoop/shell/StopJobFunction.java 790c522 > shell/src/main/java/org/apache/sqoop/shell/SubmitCommand.java PRE-CREATION > shell/src/main/java/org/apache/sqoop/shell/SubmitJobFunction.java > PRE-CREATION > shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java dd075d7 > shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java 176833a > shell/src/main/java/org/apache/sqoop/shell/core/Constants.java 44d5920 > shell/src/main/resources/shell-resource.properties 0e63c50 > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java > af31769 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java > 36f7443 > tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryDumpTool.java > c219e68 > tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java > 64b08fc > > Diff: https://reviews.apache.org/r/27330/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
