Skip to content

Conversation

emasab
Copy link
Contributor

@emasab emasab commented Jun 10, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings June 10, 2025 12:58
@emasab emasab requested a review from a team as a code owner June 10, 2025 12:58
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes a deprecation warning by using CURLOPT_PROTOCOLS_STR for newer libcurl versions while preserving compatibility for older distributions, and adds a shell override in the Makefile.

  • Use CURLOPT_PROTOCOLS_STR for curl ≥ 7.85.0, fallback to CURLOPT_PROTOCOLS otherwise
  • Set SHELL = bash in the Makefile for consistent shell behavior

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/rdhttp.c Add version‐gated switch from CURLOPT_PROTOCOLS to CURLOPT_PROTOCOLS_STR
Makefile Explicitly set the Makefile shell to bash
Comments suppressed due to low confidence (2)

src/rdhttp.c:146

  • The date ‘06/10/2025’ in the comment can be ambiguous (MM/DD vs DD/MM). Use ISO 8601 (e.g. 2025-06-10) and/or reference a specific curl version or issue tracker for when to remove the conditional.
 * older CURL versions, remove this condition once they're not supported

Makefile:12

  • Hard-coding SHELL = bash may break on systems without bash or with it in a non-standard path. Consider checking for bash availability or default to /bin/sh if bash-specific features aren’t required.
SHELL	= bash

Copy link
Contributor

@milindl milindl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. I checked documentation to ascertain that the older one is deprecated.

@emasab emasab merged commit ee55d94 into master Jun 11, 2025
1 of 2 checks passed
@emasab emasab deleted the dev_remove_curl_warning_and_makefile_fix branch June 11, 2025 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants