[GitHub] drill issue #1235: DRILL-6336: Inconsistent method name.

2018-04-29 Thread vrozov 
Github user vrozov commented on the issue:

https://github.com/apache/drill/pull/1235
  
My take is that "append" is more common for classes with the similar 
functionality, see for example `ToStringBuilder`. As there is no added benefit 
of using "print" vs "append", my recommendation is to keep "append" as is and 
see if `DebugStringBuilder` can be replaced with the `ToStringBuilder`.


---

[GitHub] drill issue #1235: DRILL-6336: Inconsistent method name.

2018-04-29 Thread arina-ielchiieva 
Github user arina-ielchiieva commented on the issue:

https://github.com/apache/drill/pull/1235
  
@vrozov so you suggest to leave as is, correct?
@paul-rogers since you have originally added `DebugStringBuilder`, do you 
agree?


---

[GitHub] drill issue #1235: DRILL-6336: Inconsistent method name.

2018-04-23 Thread vrozov 
Github user vrozov commented on the issue:

https://github.com/apache/drill/pull/1235
  
There is no need to expose implementation details as part of the class API. 
Whether `DebugStringBuilder` uses `PrintWriter.print()` or something else to 
implement `append()` must be hidden from `DebugStringBuilder` consumers. Method 
name should never depend on details of implementation as the implementation may 
change, but API should not.


---

[GitHub] drill issue #1235: DRILL-6336: Inconsistent method name.

2018-04-23 Thread BruceKuiLiu 
Github user BruceKuiLiu commented on the issue:

https://github.com/apache/drill/pull/1235
  
Thanks.


---

[GitHub] drill issue #1235: DRILL-6336: Inconsistent method name.

2018-04-23 Thread arina-ielchiieva 
Github user arina-ielchiieva commented on the issue:

https://github.com/apache/drill/pull/1235
  
@BruceKuiLiu it is not public interface, no need to deprecate the method. 
You can simply replace.


---