douglasdennis opened a new pull request, #787:
URL: https://github.com/apache/sedona/pull/787

   
   ## Did you read the Contributor Guide?
   
   - Yes, I have read [Contributor 
Rules](https://sedona.apache.org/latest-snapshot/community/rule/) and 
[Contributor Development 
Guide](https://sedona.apache.org/latest-snapshot/community/develop/)
   
   ## Is this PR related to a JIRA ticket?
   
   - Yes, the URL of the associated JIRA ticket is 
https://issues.apache.org/jira/browse/SEDONA-229.
   
   ## What changes were proposed in this PR?
   
   This PR disables the scalastyle check called MultipleStringLiteralsChecker. 
This check is responsible for detecting multiple occurrences of the same string 
literal in the same file. From what I could gather, the reason for this check 
is to detect potential cases of "magic numbers" or constants that should be 
consolidated into a constant variable. There are three issues with this check:
   
   - There is a bug in the check which flags fragments of interpolated strings 
as duplicates even though the string as a whole is not a duplicate.
   - The main offender for this check are the tests. This is because of very 
common column names like "geom". The mass majority of these checks are false 
alerts. There was one instance of this check flagging non-test code and it 
provided little value to fix it.
   - Spark SQL uses a lot of strings and I wouldn't call it bad practice to do 
so.
   
   This check did reveal some refactor points in the tests, but I would think 
that folks could already see that the tests could use some refactoring. In 
other words, this check doesn't flag anything that isn't obvious without it.
   
   I did give a go at removing all duplicate strings for the sedona-sql module 
and it led to some really ugly constant declarations in the test classes. Some 
nice refactors of the tests did fall out, and I will try to PR those once the 
linter messages are cleaned up.
   
   ## How was this patch tested?
   
   Ran the standard build process. This doesn't change any actual code.
   
   ## Did this PR include necessary documentation updates?
   
   - No, this PR does not affect any public API so no need to change the docs.
   


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

Reply via email to