You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/04/01 06:58:41 UTC
[GitHub] [drill] DanielT504 opened a new pull request #2195: DRILL-7878 Fix LGTM errors
DanielT504 opened a new pull request #2195:
URL: https://github.com/apache/drill/pull/2195
# [DRILL-7878](https://issues.apache.org/jira/browse/DRILL-7878): Changed URLs from HTTP to secure HTTPS links
## Description
Changed 2 links from HTTP to HTTPS to fix LGTM errors. 2 other links that were not valid got commented with an LGTM alert suppression line to fix the error.
## Documentation
none
## Testing
Made sure updated URLs were still supported; 2 weren't, and so did not get changed, but were still eliminated as LGTM errors.
--
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
[GitHub] [drill] vvysotskyi commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-812163388
The checksum checks are also nice to have. Please ensure that the authorship of the commits you use will be preserved
https://docs.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors
--
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
[GitHub] [drill] DanielT504 closed pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 closed pull request #2195:
URL: https://github.com/apache/drill/pull/2195
--
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
[GitHub] [drill] vvysotskyi commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-812102753
That ticket has a link to unfinished changes (that also includes changes you made in this PR). I propose to complete those changes in the scope of this PR.
--
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
[GitHub] [drill] cgivre commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-811999525
@DanielT504
I tried accessing https://onejar-maven-plugin.googlecode.com/svn/mavenrepo and https://scala-tools.org/repo-releases and both seem to be unavailable. Can you please verify which dependencies actually use these repos and if none, can we remove them all together? I tried both `http` and `https` and neither seemed to work.
Thanks!
--
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
[GitHub] [drill] DanielT504 closed pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 closed pull request #2195:
URL: https://github.com/apache/drill/pull/2195
--
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
[GitHub] [drill] DanielT504 edited a comment on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 edited a comment on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-812150512
> That ticket has a link to unfinished changes (that also includes changes you made in this PR). I propose to complete those changes in the scope of this PR.
There appears to be changes in that ticket that involve many other issues, not just non-https URLs. Should I include all of these changes in my new PR, or just the ones involving non-https URLs?
--
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
[GitHub] [drill] DanielT504 commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-812095605
> @DanielT504 please take a look at https://issues.apache.org/jira/browse/DRILL-7270 and the branch attached in the comments and please cooperate with those changes if possible.
I'm not sure I fully understand what you are requesting. Should I make a different PR for that issue instead?
--
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
[GitHub] [drill] luocooong commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-812993996
@cgivre @vvysotskyi Hello. I try to using the following commands on my local, But had not found any jar related to the above repository. So, Is there any new revision suggestions? Or ready to merge it?
1. the output if using a repository. eg `jitpack.io` :
```
find /Users/luoc/.m2/ -name _remote.repositories |xargs grep -ri "jitpack.io" -l
/Users/luoc/.m2//repository/com/github/vvysotskyi/drill-calcite/calcite/1.21.0-drill-r2/_remote.repositories
/Users/luoc/.m2//repository/com/github/vvysotskyi/drill-calcite/calcite-core/1.20.0-drill-r2/_remote.repositories
...
```
2. no output with the `onejar-maven-plugin.googlecode.com` and `scala-tools.org` :
```
find /Users/luoc/.m2/ -name _remote.repositories |xargs grep -ri "onejar-maven-plugin.googlecode.com" -l
```
--
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
[GitHub] [drill] cgivre commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-816710255
@DanielT504
Can you please rebase with current master so that we can merge this PR?
Thx!
--
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
[GitHub] [drill] cgivre commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-812018074
@DanielT504
I approved this, but can you take a look at some of the `pom.xml` files in the `contrib` folder to see if there are any other repos that use `http`? Assuming this passes CI, we are good to go.
Thanks for the contribution!!
--
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
[GitHub] [drill] luocooong commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-817154152
@cgivre @DanielT504 It seems that this PR is related to an umbrella JIRA (DRILL-7878). So, It better to convert this PR as a new sub-task for the DRILL-7878 (It no longer represents the 7828)? Maybe it can speed up the merge for this PR.
--
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
[GitHub] [drill] DanielT504 commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-817199033
@luocooong Just to be clear, does this just mean creating another JIRA subtask, then editing the PR description and commit titles to match the ticket code?
--
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
[GitHub] [drill] DanielT504 commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-813708670
> The checksum checks are also nice to have. Please ensure that the authorship of the commits you use will be preserved
> https://docs.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors
It seems that dgrinchenko does not have a public email for me to link for co-authorship, and I have no way to contact him. How should I proceed with the changes?
--
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
[GitHub] [drill] cgivre merged pull request #2195: DRILL-7270 Changed URLs from HTTP to secure HTTPS links and added checksums
Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2195:
URL: https://github.com/apache/drill/pull/2195
--
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
[GitHub] [drill] cgivre commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-815828658
@DanielT504
How's it going? I was on vacation last week, but where are we on this PR?
--
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
[GitHub] [drill] DanielT504 commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-812018958
> @DanielT504
> I approved this, but can you take a look at some of the `pom.xml` files in the `contrib` folder to see if there are any other repos that use `http`? Assuming this passes CI, we are good to go.
> Thanks for the contribution!!
I will, thank you!
--
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
[GitHub] [drill] DanielT504 removed a comment on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 removed a comment on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-813708670
> The checksum checks are also nice to have. Please ensure that the authorship of the commits you use will be preserved
> https://docs.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors
It seems that dgrinchenko does not have a public email for me to link for co-authorship, and I have no way to contact him. How should I proceed with the changes?
--
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
[GitHub] [drill] cgivre commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-817231551
> @luocooong Just to be clear, does this just mean creating another JIRA subtask, then editing the PR description and commit titles to match the ticket code?
This is just done in JIRA. I'll merge this tomorrow morning unless there are any objections.
--
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
[GitHub] [drill] DanielT504 commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-815924539
> @DanielT504
> How's it going? I was on vacation last week, but where are we on this PR?
No problem, I have made the changes @vvysotskyi asked for, and credited @dgrinchenko as co-author. 2 of the Git CI tests simply timed out after the 90 minute limit, which leads me to believe that there is no issue with what I changed (since I only updated pom.xml files and they would have failed the checks much sooner), but I am not certain.
--
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
[GitHub] [drill] luocooong edited a comment on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
luocooong edited a comment on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-817154152
@cgivre @DanielT504 It seems that this PR is related to an umbrella JIRA (DRILL-7878). So, It better to convert this PR as a new sub-task for the DRILL-7878 (It no longer represents the 7878)? Maybe it can speed up the merge for this PR.
--
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
[GitHub] [drill] DanielT504 commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-817233079
@cgivre none from me, thank you!
--
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
[GitHub] [drill] DanielT504 commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-812150512
> That ticket has a link to unfinished changes (that also includes changes you made in this PR). I propose to complete those changes in the scope of this PR.
There appears to be changes in that ticket that involve checksum-checks, not just non-https URLs. Should I include all of these changes in my new PR, or just the ones involving non-https URLs?
--
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
[GitHub] [drill] DanielT504 commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-812009637
> @DanielT504
> I tried accessing https://onejar-maven-plugin.googlecode.com/svn/mavenrepo and https://scala-tools.org/repo-releases and both seem to be unavailable. Can you please verify which dependencies actually use these repos and if none, can we remove them all together? I tried both `http` and `https` and neither seemed to work.
> Thanks!
Good point, I've removed those ones entirely.
--
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
[GitHub] [drill] DanielT504 commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
DanielT504 commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-816773318
> @DanielT504
> Can you please rebase with current master so that we can merge this PR?
> Thx!
I had some complications because I had to revert a revert to mend a prior error on my part. Could you double check that this is what you were asking for?
--
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
[GitHub] [drill] vvysotskyi commented on pull request #2195: DRILL-7878 Fix LGTM errors
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2195:
URL: https://github.com/apache/drill/pull/2195#issuecomment-813008264
Hi @luocooong, no, it is not ready. Please see my comments above regarding cooperating with changes from another ticket...
--
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