Github user dsbos commented on a diff in the pull request:
https://github.com/apache/drill/pull/93#discussion_r35054916
--- Diff:
common/src/main/java/org/apache/drill/common/config/DrillConfig.java ---
@@ -138,34 +154,83 @@ public static DrillConfig create(Properties
testConfigurations) {
return create(null, testConfigurations, true);
}
- public static DrillConfig create(String overrideFileName, boolean
enableServerConfigs) {
- return create(overrideFileName, null, enableServerConfigs);
+ /**
+ * ...
+ * @param overrideFileResourcePathname
+ * see {@link #create(String)}'s {@code
overrideFileResourcePathname}
+ */
+ public static DrillConfig create(String overrideFileResourcePathname,
boolean enableServerConfigs) {
+ return create(overrideFileResourcePathname, null, enableServerConfigs);
}
- private static DrillConfig create(String overrideFileName, Properties
overriderProps, boolean enableServerConfigs) {
- overrideFileName = overrideFileName == null ?
CommonConstants.CONFIG_OVERRIDE : overrideFileName;
-
- // first we load defaults.
+ /**
+ * ...
--- End diff --
[Reply via e-mail because I don't currently see this comment of yours on
the pull-request page.]
> > + /**
> > + * ...xx
> I haven't seen this convention before, but I assume this means that the
docs for similar methods suffice and don't need to be repeated here.
No, actually it was an attempt to indicate that I hadn't addressed the
method description (the part before the parameters), and not intended to mean
"otherwise same as above".
I was trying to quickly at least document the parameter (add the reference
to the other existing text) without taking time to fill in the whole
documentation comment.
Would simply leaving the method description part blank be better than using
"..."?
Daniel
Jason Altekruse wrote:
>
> In common/src/main/java/org/apache/drill/common/config/DrillConfig.java
<https://github.com/apache/drill/pull/93#discussion_r35042277>:
>
> > }
> >
> > - private static DrillConfig create(String overrideFileName,
Properties overriderProps, boolean enableServerConfigs) {
> > - overrideFileName = overrideFileName == null ?
CommonConstants.CONFIG_OVERRIDE : overrideFileName;
> > -
> > - // first we load defaults.
> > + /**
> > + * ...
>
> I haven't seen this convention before, but I assume this means that the
docs for similar methods suffice and don't need to be repeated here. I would
possibly recommend moving the longer comment above to here, in which case I
think it would be possible to just put "..." above all of the alternative
implementations of this method. I think it makes the most sense to put the
comment describing the general idea of the functions purpose above the version
that also describes what all of parameters can do.
>
> â
> Reply to this email directly or view it on GitHub
<https://github.com/apache/drill/pull/93/files#r35042277>.
>
--
Daniel Barclay
MapR Technologies
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
