[jira] Commented: (PIG-1370) Marking Pig interfaces for org.apache.pig package
[ https://issues.apache.org/jira/browse/PIG-1370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12856496#action_12856496 ] Pradeep Kamath commented on PIG-1370: - bq. But it is accepted as an arg to one of ResourceSchema's constructors. I think that makes it public, unless we want to say that constructor isn't intended for public use (in which case, why is it public?). This constructor is called from internal Pig code and we should not expose this to users - if we don't make the constructor public we cannot call this constructor since the callers are in different packages - I really think we need an annotation to say internal-use so we can annotate some of the "public" methods which we don't want users to use. bq. I did mark ComparisonFunc as deprecated. Are you saying we should just remove it instead of deprecate it? I think for now deprecated is fine. > Marking Pig interfaces for org.apache.pig package > - > > Key: PIG-1370 > URL: https://issues.apache.org/jira/browse/PIG-1370 > Project: Pig > Issue Type: Sub-task > Components: documentation >Reporter: Alan Gates >Assignee: Alan Gates > Fix For: 0.8.0 > > Attachments: PIG-1370.patch, PIG-1370_2.patch > > > Done as a separate JIRA from PIG-1311 since this alone contains quite a lot > of changes. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (PIG-1370) Marking Pig interfaces for org.apache.pig package
[ https://issues.apache.org/jira/browse/PIG-1370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12856246#action_12856246 ] Alan Gates commented on PIG-1370: - bq. In Expression.java the comment reads - "// Because this isn't actually used yet. " - Expression class is currently used in LoadMetadata.setPartitionFilter() - I agree though that we should mark this not stable - Evolving perhaps? Changed to Evolving, which matches LoadMetadata. bq. In SortColInfo.java, I think we should not mark this public since this is used entirely inside Pig code and any communicating to external storeFuncs is through ResourceSchema. The same comment holds for SortInfo.java But it is accepted as an arg to one of ResourceSchema's constructors. I think that makes it public, unless we want to say that constructor isn't intended for public use (in which case, why is it public?). bq. Is there a way to mark public methods in a class as "internal" - for example PigServer.getAliases() - currently this is used by unit tests - should we be exposing this method to the user? if not can we mark it not public through annotation? (Is there a different policy like if there is no javadoc comments for a public method, then it is not truely public?) I don't know how to do this with annotations. I've changed the javadocs to have an initial sentence of "Intended to be used by unit tests only." bq. I think CollectableLoadFunc should be evolving Motion carried, I've changed it to evolving. bq. ComparsionFunc.java unfortunately already had ctrl-m chars - the new additions in the patch also do - if it isn't extensive, we could remove the ctrl-m chars. Another comment is that per http://wiki.apache.org/pig/Pig070IncompatibleChanges, custom comparators are no longer supported since 0.7 - if so, should this be @deprecated? - I think currently custom comparators don't work in local mode. I did mark ComparisonFunc as deprecated. Are you saying we should just remove it instead of deprecate it? bq. I hope marking LoadFunc stable will not prevent additions to this abstract class (which should not break backward compatibilty if default impls are provided) The definition of stable is that it will work across versions without a recompile of Java code. So this won't prevent growing existing abstract classes. > Marking Pig interfaces for org.apache.pig package > - > > Key: PIG-1370 > URL: https://issues.apache.org/jira/browse/PIG-1370 > Project: Pig > Issue Type: Sub-task > Components: documentation >Reporter: Alan Gates >Assignee: Alan Gates > Fix For: 0.8.0 > > Attachments: PIG-1370.patch > > > Done as a separate JIRA from PIG-1311 since this alone contains quite a lot > of changes. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (PIG-1370) Marking Pig interfaces for org.apache.pig package
[ https://issues.apache.org/jira/browse/PIG-1370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12856118#action_12856118 ] Pradeep Kamath commented on PIG-1370: - - In Expression.java the comment reads - "// Because this isn't actually used yet. " - Expression class is currently used in LoadMetadata.setPartitionFilter() - I agree though that we should mark this not stable - Evolving perhaps? - In SortColInfo.java, I think we should not mark this public since this is used entirely inside Pig code and any communicating to external storeFuncs is through ResourceSchema. The same comment holds for SortInfo.java - In EvalFunc.java minor typo - "* a fields " should be "* fields" - Is there a way to mark public methods in a class as "internal" - for example PigServer.getAliases() - currently this is used by unit tests - should we be exposing this method to the user? if not can we mark it not public through annotation? (Is there a different policy like if there is no javadoc comments for a public method, then it is not truely public?) - In Accumulator.java minor typo - "This in intended" should be "This is intended" - I think CollectableLoadFunc should be evolving - ComparsionFunc.java unfortunately already had ctrl-m chars - the new additions in the patch also do - if it isn't extensive, we could remove the ctrl-m chars. Another comment is that per http://wiki.apache.org/pig/Pig070IncompatibleChanges, custom comparators are no longer supported since 0.7 - if so, should this be @deprecated? - I think currently custom comparators don't work in local mode. - I hope marking LoadFunc stable will not prevent additions to this abstract class (which should not break backward compatibilty if default impls are provided) > Marking Pig interfaces for org.apache.pig package > - > > Key: PIG-1370 > URL: https://issues.apache.org/jira/browse/PIG-1370 > Project: Pig > Issue Type: Sub-task > Components: documentation >Reporter: Alan Gates >Assignee: Alan Gates > Fix For: 0.8.0 > > Attachments: PIG-1370.patch > > > Done as a separate JIRA from PIG-1311 since this alone contains quite a lot > of changes. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (PIG-1370) Marking Pig interfaces for org.apache.pig package
[ https://issues.apache.org/jira/browse/PIG-1370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12855384#action_12855384 ] Hadoop QA commented on PIG-1370: +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12441251/PIG-1370.patch against trunk revision 932161. +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/292/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/292/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/292/console This message is automatically generated. > Marking Pig interfaces for org.apache.pig package > - > > Key: PIG-1370 > URL: https://issues.apache.org/jira/browse/PIG-1370 > Project: Pig > Issue Type: Sub-task > Components: documentation >Reporter: Alan Gates >Assignee: Alan Gates > Fix For: 0.8.0 > > Attachments: PIG-1370.patch > > > Done as a separate JIRA from PIG-1311 since this alone contains quite a lot > of changes. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
