Skip to content

Conversation

@iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Jun 20, 2025

User description

🔗 Related Issues

💥 What does this PR do?

This pull request introduces deprecation warnings and updates related to FTP proxy support in the Proxy class, as well as corresponding adjustments in the test suite. The changes aim to mark FTP proxy functionality as deprecated, with plans for removal in future versions, while maintaining backward compatibility for now.

Deprecation of FTP Proxy Support:

Updates to Test Suite:

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Other


Description

• Deprecate FTP proxy support in .NET Proxy class
• Add Obsolete attributes and documentation warnings
• Update test suite with deprecation comments
• Maintain backward compatibility during transition


Changes walkthrough 📝

Relevant files
Enhancement
Proxy.cs
Deprecate FTP proxy support with obsolete attributes         

dotnet/src/webdriver/Proxy.cs

• Added [Obsolete] attribute to FTP proxy field and property
• Added
deprecation remarks to FtpProxy property documentation
• Added TODO
comments referencing GitHub issue for future removal
• Maintained
backward compatibility while marking as deprecated

+9/-0     
Tests
ProxyTest.cs
Update tests with FTP proxy deprecation comments                 

dotnet/test/common/ProxyTest.cs

• Added TODO comments before all FTP proxy test assertions

Referenced GitHub issue for future removal in comments
• Added extra
newline for formatting consistency
• Maintained existing test
functionality while documenting deprecation

+36/-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-dotnet .NET Bindings label Jun 20, 2025
    @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ❌

    1234 - Not compliant

    Non-compliant requirements:

    • Fix JavaScript execution in link's href attribute when using click() method
    • Ensure compatibility with Firefox 42.0
    • Restore functionality that worked in version 2.47.1 but broke in 2.48.0/2.48.2

    5678 - Not compliant

    Non-compliant requirements:

    • Fix ChromeDriver ConnectFailure error on Ubuntu 16.04.4
    • Resolve connection refused errors for subsequent ChromeDriver instances
    • Ensure proper ChromeDriver initialization beyond first instance

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

    Unrelated Changes

    The PR deprecates FTP proxy functionality but doesn't address the reported JavaScript click() issues or ChromeDriver connection problems from the linked tickets. This appears to be unrelated maintenance work.

    [Obsolete("FTP proxy support is deprecated and will be removed in a future version.")]
    private string? ftpProxyLocation;

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 20, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Remove extra blank line

    Remove the extra blank line between the using statements and namespace
    declaration. This maintains consistent code formatting and follows C# style
    conventions.

    dotnet/test/common/ProxyTest.cs [20-24]

     using NUnit.Framework;
     using System.Collections.Generic;
     
    -
     namespace OpenQA.Selenium;
    • Apply / Chat
    Suggestion importance[1-10]: 3

    __

    Why: The suggestion correctly identifies an unnecessary blank line added at line 23 in the PR. Removing it is a valid stylistic improvement for code consistency, but it has a minor impact on the overall code quality.

    Low
    • Update

    Copy link
    Member

    @nvborisenko nvborisenko left a comment

    Choose a reason for hiding this comment

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

    Thanks for your help!

    Please remain one line deprecation, that's it.

    @iampopovich iampopovich force-pushed the feat/deprecate-ftp-proxy-cs branch from 4c74d4a to 8f65de2 Compare June 20, 2025 12:54
    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.

    3 participants