Github user jaltekruse commented on a diff in the pull request:
https://github.com/apache/drill/pull/93#discussion_r35268073
--- 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 --
I think if you should just put in explicit todo. However I'm not even sure
that it makes sense to document these methods individually. There is a comment
that is very descriptive about what all of these do, it's just above the wrong
version of the method. If you move it above the one that takes all of the
parameters, I think it would be fine to even leave out the parameter
descriptions on the other versions and have a common message directing people
to the one descriptive comment (which also describes all possible parameters).
No need for a todo, not a bunch of work up front.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
