You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/12/02 15:53:14 UTC
[GitHub] [iceberg] Fokko opened a new pull request, #6348: Python: Update license-checker
Fokko opened a new pull request, #6348:
URL: https://github.com/apache/iceberg/pull/6348
So that we have the same as in Iceberg
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #6348: Python: Update license-checker
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #6348:
URL: https://github.com/apache/iceberg/pull/6348#issuecomment-1356908698
@Fokko, what do you think about using the same license check script (copied) that we do in the Java version?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #6348: Python: Update license-checker
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #6348:
URL: https://github.com/apache/iceberg/pull/6348#discussion_r1044686299
##########
python/dev/.rat-excludes:
##########
@@ -0,0 +1,2 @@
+.rat-excludes
Review Comment:
We can keep the two separate but use the same script right? I'd prefer that since it works well and is easy to verify.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #6348: Python: Update license-checker
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #6348:
URL: https://github.com/apache/iceberg/pull/6348#discussion_r1044923603
##########
python/dev/.rat-excludes:
##########
@@ -0,0 +1,2 @@
+.rat-excludes
Review Comment:
Why do we need to copy it? We could just commit a copy of it in place of the current script.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #6348: Python: Update license-checker
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #6348:
URL: https://github.com/apache/iceberg/pull/6348#discussion_r1044764109
##########
python/dev/.rat-excludes:
##########
@@ -0,0 +1,2 @@
+.rat-excludes
Review Comment:
This will make the building more complicated. Before doing a `poetry build`, we need to copy the script from the parent directory to the Python directory.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #6348: Python: Update license-checker
Posted by GitBox <gi...@apache.org>.
nastra commented on code in PR #6348:
URL: https://github.com/apache/iceberg/pull/6348#discussion_r1039781132
##########
python/dev/.rat-excludes:
##########
@@ -0,0 +1,2 @@
+.rat-excludes
Review Comment:
related to [this old comment](https://github.com/apache/iceberg/pull/5840#discussion_r978666139) I'm curious whether it would now make sense to re-use the `check-license` script from the root dev folder
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #6348: Python: Update license-checker
Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #6348:
URL: https://github.com/apache/iceberg/pull/6348#discussion_r1042617131
##########
python/dev/.rat-excludes:
##########
@@ -0,0 +1,2 @@
+.rat-excludes
Review Comment:
I'd rather keep the two projects isolated so we have the possibility to split pyiceberg out in a separate repository.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #6348: Python: Update license-checker
Posted by GitBox <gi...@apache.org>.
rdblue merged PR #6348:
URL: https://github.com/apache/iceberg/pull/6348
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on pull request #6348: Python: Update license-checker
Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #6348:
URL: https://github.com/apache/iceberg/pull/6348#issuecomment-1368832789
@rdblue I missed your comment here. The one is exactly the same as the Java one:
```
➜ iceberg git:(fd-copy-rat-from-iceberg) diff dev/check-license python/dev/check-license
43c43
< if [ $? -ne 0 ]; then
---
> if [ $? -ne 0 ]; then
80c80
< if test ! -z "$ERRORS"; then
---
> if test ! -z "$ERRORS"; then
84c84
< else
---
> else
```
The difference is in the trailing whitespace on the Java one. By re-using I was under the impression that we don't want to have two identical scripts in the repository.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org