Skip to content

[rb] Feat 15905/deprecate ftp proxy #15926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Jun 20, 2025

User description

🔗 Related Issues

fixes #15905

💥 What does this PR do?

This pull request introduces a deprecation notice for FTP proxy support in the ftp= method of the Proxy class in rb/lib/selenium/webdriver/common/proxy.rb.

Deprecation notice:

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Other


Description

  • Add deprecation warning for FTP proxy support in Ruby bindings

  • Use WebDriver logger to notify users of upcoming removal


Changes walkthrough 📝

Relevant files
Deprecation
proxy.rb
Add FTP proxy deprecation warning                                               

rb/lib/selenium/webdriver/common/proxy.rb

  • Added deprecation warning using WebDriver.logger.deprecate() in ftp=
    method
  • Warning indicates FTP proxy support will be removed in future versions
  • +1/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added the C-rb Ruby Bindings label Jun 20, 2025
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Context

    The deprecation warning lacks information about when the feature will be removed or what alternatives users should consider. The second parameter is nil which could provide version information.

    WebDriver.logger.deprecate('FTP proxy support', nil, id: :ftp_proxy)
    self.type = :manual

    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 20, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Specify deprecation timeline

    The deprecation warning should specify when the feature will be removed to help
    users plan their migration. Consider adding a version number or timeline to make
    the deprecation more actionable.

    rb/lib/selenium/webdriver/common/proxy.rb [79-83]

     def ftp=(value)
    -  WebDriver.logger.deprecate('FTP proxy support', nil, id: :ftp_proxy)
    +  WebDriver.logger.deprecate('FTP proxy support', 'selenium-webdriver 5.0', id: :ftp_proxy)
       self.type = :manual
       @ftp = value
     end
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies that the deprecation warning is missing a timeline for removal. Providing a target version, as shown in the improved_code, makes the warning more actionable for developers and is a best practice for managing deprecations.

    Medium
    • Update

    @Delta456 Delta456 requested a review from aguspe June 21, 2025 11:40
    @aguspe aguspe merged commit 4d99963 into SeleniumHQ:trunk Jun 22, 2025
    23 of 24 checks passed
    @aguspe
    Copy link
    Contributor

    aguspe commented Jun 22, 2025

    Thank you for your contribution @iampopovich !

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    [🚀 Feature]: Deprecate/Remove FTP Proxy support from all bindings
    3 participants