Skip to content
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

[dotnet] Propagate IWebDriver.GetAttribute obsoletion to WebDriver #14802

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

RenderMichael
Copy link
Contributor

@RenderMichael RenderMichael commented Nov 25, 2024

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

The IWebDriver.GetAttribute method was recently depreciated. This change propagates the obsoletion to the concrete implementations.

Motivation and Context

Anyone using WebDriver directly (which has been my default experience, especially thanks to the var keyword) will not see the warnings.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

enhancement


Description

  • Marked the GetAttribute method as obsolete in both EventFiringWebDriver and WebElement classes.
  • Provided a message suggesting the use of GetDomAttribute or GetDomProperty as alternatives.
  • Indicated that GetAttribute will be removed in Selenium 6.

Changes walkthrough 📝

Relevant files
Enhancement
EventFiringWebDriver.cs
Mark `GetAttribute` method as obsolete in EventFiringWebDriver

dotnet/src/support/Events/EventFiringWebDriver.cs

  • Marked GetAttribute method as obsolete.
  • Provided alternatives in the obsolete message.
  • +1/-0     
    WebElement.cs
    Mark `GetAttribute` method as obsolete in WebElement         

    dotnet/src/webdriver/WebElement.cs

  • Marked GetAttribute method as obsolete.
  • Provided alternatives in the obsolete message.
  • +1/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    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

    Documentation
    The obsolete attribute message should clarify if both alternative methods need to be called or if they serve different purposes and the user should choose one based on their needs

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Align variable initialization with documented method return value specification

    Initialize attribute variable to null instead of empty string to match the
    documented return value in the method summary

    dotnet/src/support/Events/EventFiringWebDriver.cs [1619]

    -string attribute = string.Empty;
    +string attribute = null;
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion correctly identifies a discrepancy between the method's documentation (which states it returns null if value is not set) and the implementation that initializes with string.Empty. This change improves consistency with the documented behavior.

    7

    💡 Need additional feedback ? start a PR chat

    @nvborisenko
    Copy link
    Member

    Good catch, thank you!

    @nvborisenko nvborisenko merged commit b3ef505 into SeleniumHQ:trunk Nov 25, 2024
    1 check passed
    @RenderMichael RenderMichael deleted the get-attribute-obsolete branch November 25, 2024 19:51
    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.

    2 participants