busbey commented on a change in pull request #86:
URL: https://github.com/apache/hbase-connectors/pull/86#discussion_r733707886



##########
File path: thrift-python/README.md
##########
@@ -0,0 +1,169 @@
+# Project description

Review comment:
       Individual files need to have a header that points out the licensing. 
see here for more info:
   
   https://apache.org/legal/src-headers.html

##########
File path: thrift-python/LICENSE
##########
@@ -0,0 +1,202 @@
+
+                                 Apache License
+                           Version 2.0, January 2004
+                        http://www.apache.org/licenses/
+

Review comment:
       Can we avoid having another copy of the LICENSE here? the top level one 
is meant to cover the source in the repo. if we need a copy for e.g. inclusion 
in some artifact can we pull that one?

##########
File path: thrift-python/setup.py
##########
@@ -0,0 +1,44 @@
+"""
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+from setuptools import setup, find_packages
+from os import path
+from sys import version_info
+here = path.abspath(path.dirname(__file__))
+if version_info.major == 3:
+    with open(path.join(here, 'README.md'), encoding='utf-8') as f:
+        long_description = f.read()
+else:
+    with open(path.join(here, 'README.md')) as f:
+        long_description = f.read()
+
+setup(
+    name='thbase',
+    version='1.3.6',

Review comment:
       So far we've gone with unified releases from hbase-connectors. That is, 
the version here should be the same as the hbase-connectors version it is 
released in. ATM the in development version is 1.0.1. With this library adding 
we should probably bump that to 1.1 or 2.0.

##########
File path: thrift-python/setup.py
##########
@@ -0,0 +1,44 @@
+"""
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+from setuptools import setup, find_packages
+from os import path
+from sys import version_info
+here = path.abspath(path.dirname(__file__))
+if version_info.major == 3:
+    with open(path.join(here, 'README.md'), encoding='utf-8') as f:
+        long_description = f.read()
+else:
+    with open(path.join(here, 'README.md')) as f:
+        long_description = f.read()
+
+setup(
+    name='thbase',
+    version='1.3.6',
+    description='HBase thrift2 client python API. Compatible with python2.7 
and python3',
+    long_description=long_description,
+    long_description_content_type='text/markdown',
+    # Author details
+    author='',
+    author_email='',
+    url='',
+    classifiers=[
+        "License :: OSI Approved :: Apache Software License",
+        "Natural Language :: English",
+        "Programming Language :: Python :: 2.7",

Review comment:
       how long are we looking to maintain python 2.7 support? is it still 
commonly needed?

##########
File path: thrift-python/setup.py
##########
@@ -0,0 +1,44 @@
+"""
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+from setuptools import setup, find_packages
+from os import path
+from sys import version_info
+here = path.abspath(path.dirname(__file__))
+if version_info.major == 3:
+    with open(path.join(here, 'README.md'), encoding='utf-8') as f:
+        long_description = f.read()
+else:
+    with open(path.join(here, 'README.md')) as f:
+        long_description = f.read()
+
+setup(
+    name='thbase',

Review comment:
       I'd really like it if the name was less opaque. maybe `hbase-thrift2` or 
`hbase-client-thrift2`?

##########
File path: thrift-python/setup.py
##########
@@ -0,0 +1,44 @@
+"""
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+from setuptools import setup, find_packages
+from os import path
+from sys import version_info
+here = path.abspath(path.dirname(__file__))
+if version_info.major == 3:
+    with open(path.join(here, 'README.md'), encoding='utf-8') as f:
+        long_description = f.read()
+else:
+    with open(path.join(here, 'README.md')) as f:
+        long_description = f.read()
+
+setup(
+    name='thbase',
+    version='1.3.6',
+    description='HBase thrift2 client python API. Compatible with python2.7 
and python3',
+    long_description=long_description,
+    long_description_content_type='text/markdown',
+    # Author details
+    author='',
+    author_email='',

Review comment:
       these should be set to something like "Apache HBase Community" and 
"[email protected]"

##########
File path: thrift-python/setup.py
##########
@@ -0,0 +1,44 @@
+"""
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+from setuptools import setup, find_packages
+from os import path
+from sys import version_info
+here = path.abspath(path.dirname(__file__))
+if version_info.major == 3:
+    with open(path.join(here, 'README.md'), encoding='utf-8') as f:
+        long_description = f.read()
+else:
+    with open(path.join(here, 'README.md')) as f:
+        long_description = f.read()
+
+setup(
+    name='thbase',
+    version='1.3.6',
+    description='HBase thrift2 client python API. Compatible with python2.7 
and python3',

Review comment:
       for the module description we should use the proper trademark "Apache 
HBase thrift2 client ..."




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