Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/7872#discussion_r36244315
  
    --- Diff: 
yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnSparkHadoopUtil.scala ---
    @@ -244,6 +251,47 @@ object YarnSparkHadoopUtil {
         }
       }
     
    +  /**
    +   * Escapes a string for inclusion in a command line executed by Yarn. 
Yarn executes commands
    +   * using a batch file (.cmd) Windows escaping is neccessary. The
    +   * argument is enclosed in single quotes only if there are spaces inside 
the string.
    +   * Also some key characters are escaped that have a meaning in Windows.
    +   *
    +   * @param arg A single argument.
    +   * @return Argument quoted for execution via Yarn's generated shell 
script.
    +   */
    +  def escapeForShellWindows(arg: String): String = {
    --- End diff --
    
    It's fine if you have a reason to add more code. But, as I requested, 
please explain that in the change summary (if you don't know what that is, 
that's the very first comment up there under the title, which gets added to the 
git log). That way the explanation gets recorded in the change history, and 
someone later doesn't need to scratch their heads figuring out why these things 
were done this way.


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to