grundprinzip commented on PR #49443:
URL: https://github.com/apache/spark/pull/49443#issuecomment-2583056476

   > I'm not sure why you say like that with `-e`.
   > 
   > > This is a bit of a weird patch. The mvn command should actually continue 
checking for `$?` and not for the error output being empty. In particular 
because it parses for "Unformatted files"
   > > Not the second issue is that the lint-scala should not care about the 
code in
   > > ```
   > > 
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/ExecutorKubernetesCredentialsFeatureStepSuite.scala
   > > ```
   > > 
   > > 
   > >     
   > >       
   > >     
   > > 
   > >       
   > >     
   > > 
   > >     
   > >   
   > > The mvn command explicitly just checks for
   > > ```
   > >     -pl sql/api \
   > >     -pl sql/connect/common \
   > >     -pl sql/connect/server \
   > >     -pl connector/connect/client/jvm \
   > > ```
   > 
   > It's a standard way to stop script if some command exist abnormally during 
the script. Apache Spark utilizes it in many script. Are you aware of it, 
@grundprinzip ?
   > 
   > ```
   > $ git grep 'set -e' | wc -l
   >       33
   > ```
   
   yes using `-e` exits immediately when any bash command returns a non zero 
exit code. But this is even true for accessing an undefined variable or so. My 
point was that I'm not sure why the original command was changed from checking 
for `$?` to prepping the output.
   
   The main reason for checking for the error is that the goal of the script is 
to be actionable and have a good error message that the user knows what to do 
to fix the error. Just failing the command does not help the user to understand 
how to fix it.
   
   Lastly, my questions still remains why this came up for the 
`ExecutorKubernetesCredentialsFeatureStepSuite.scala` file because it's not in 
scope for the scalastyle check.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to