Skip to content

Conversation

@pezia
Copy link

@pezia pezia commented Feb 3, 2020

No description provided.

@PRMerger12
Copy link
Contributor

@pezia : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@pezia pezia changed the title Fixed typo in Alpine download URLs Fixed typo in Alpine download URLs in ODBC Driver installation page Feb 3, 2020
@ktoliver ktoliver added the aq-pr-triaged tracking label for the PR review team label Feb 3, 2020
@pezia
Copy link
Author

pezia commented Feb 3, 2020

Maybe this could be improved further by adding -O to the curl commands to actually save the files.

@MightyPen
Copy link
Contributor

@David-Engel Hi again David, Can you pass judgment on this PR #4070, which like PR #4104 is about ODBC in the context Linux-Mac.

Thanks. G

* Added -O to curl commands to actually save the files
* Added signature check of MSSQL Tools
@PRMerger19
Copy link
Contributor

@pezia : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@MightyPen
Copy link
Contributor

@v-makouz Hello Maxim, Can you please process this customer-create "public" Pull Request #4070.
This PR proposes changes that Conflict with one of your recent PRs (your PR being dated perhaps January/31-ish).

Maxim, we need you to either (A) Resolve the conflicts here (or MS Teams chat with me while you watch me Resolve), or (B) Ask me to Close this PR and give the reason (such as Incorrect change proposal, or outdated, or...?).

Thank you. (GeneMi = MightyPen)

cc: @pezia

@MightyPen MightyPen requested review from v-makouz and removed request for MightyPen February 10, 2020 22:21
@v-makouz
Copy link
Contributor

v-makouz commented Feb 10, 2020

@MightyPen Adding -O is good (that was my bad for missing it) and the verification for the tools. But the package filename has to remain msodbcsql17_17.5.1.1-1_amd64.apk, so the name should be fixed in the "install" part.

Also the version for tools should change to 17.5.1.2 (this is a recent fix, so it wasn't in the original doc)

For the conflict, you can either resolve it here with those changes, or you can close this PR and I'll make another one with all the changes (-O, package name, tools verify, and tools version) whichever is easier

@MightyPen
Copy link
Contributor

@pezia Hello Zsolt, We like part of your PR #4070, but we do not want all parts of it.
So with our thanks to you, we are going to add the extra gpg line your PR offers, plus the important -O additions.
But for this particular PR, we are going to Close it, rather than Merge it.

Thank you for submitting this PR, it is very helpful. We hope to hear from you again.

@MightyPen MightyPen closed this Feb 10, 2020
MightyPen added a commit that referenced this pull request Feb 10, 2020
This public PR is created by GeneMi (MightyPen) and v-Makouz. This purposely follows merged public PR #4104 (PR #4070 was rejected, kinda).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants