[GitHub] [phoenix-queryserver] stoty commented on a change in pull request #39: PHOENIX-5901 Fix licensing in preparation for first python release

[GitBox <gi...@apache.org>]

stoty commented on a change in pull request #39:
URL: https://github.com/apache/phoenix-queryserver/pull/39#discussion_r441282698



##########
File path: python/LICENSE
##########
@@ -0,0 +1,202 @@
+

Review comment:
       Do we need license files in this top level directory, when we have the licenses one level down together with the real content ? 
   
   I also think that this directory should only contain the phoenixdb subdirectory, and we should remove the README.md file. It's redundant, and since it's not in the root directory of a git repo, it's not very useful in the context of github either.
   
   Perhaps we could just flatten the directory structure, and instead of /python/phoenixdb/stuff, we could have just python-phoenixdb/stuff ? 

##########
File path: python/phoenixdb/NEWS.rst
##########
@@ -1,3 +1,19 @@
+..

Review comment:
       According to http://www.apache.org/legal/src-headers.html#faq-exceptions README type .rst or .RST files don't need a license header. 
   Since these files are meant to be read without preprocessing, the license headers are inconvenient for the reader. (I am not talking about the .rst files in the doc dir, which are meant to be processed)




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org