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

[py] moved project metadata from setup.py to pyproject.toml #14311

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

sandeepsuryaprasad
Copy link
Contributor

@sandeepsuryaprasad sandeepsuryaprasad commented Jul 26, 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

  • Moved project metadata from setup.py to pyproject.toml

Motivation and Context

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

  • Moved project metadata from setup.py to pyproject.toml.
  • Removed project metadata such as name, version, license, description, URLs, classifiers, packages, and dependencies from setup.py.
  • Added project metadata including name, version, license, description, readme, Python requirements, classifiers, and dependencies to pyproject.toml.
  • Defined package inclusion and exclusion rules in pyproject.toml.
  • Added project URLs for repository, bug tracker, changelog, documentation, and source code in pyproject.toml.

Changes walkthrough 📝

Relevant files
Enhancement
setup.py
Removed project metadata from `setup.py`.                               

py/setup.py

  • Removed project metadata such as name, version, license, description,
    URLs, classifiers, packages, and dependencies.
  • Retained custom installation command and Rust extensions
    configuration.
  • +0/-57   
    pyproject.toml
    Added project metadata to `pyproject.toml`.                           

    py/pyproject.toml

  • Added project metadata including name, version, license, description,
    readme, Python requirements, classifiers, and dependencies.
  • Defined package inclusion and exclusion rules.
  • Added project URLs for repository, bug tracker, changelog,
    documentation, and source code.
  • +44/-0   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Correct the format of the RustExtension key to use a string instead of a dictionary

    The dictionary key for the RustExtension is incorrectly formatted as a dictionary
    itself. It should be a string representing the name of the extension.

    py/setup.py [32]

    -RustExtension(
    -    {"selenium-manager": "selenium.webdriver.common.selenium-manager"}
    +RustExtension("selenium-manager", "selenium.webdriver.common.selenium-manager")
     
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: The suggestion corrects a potential bug by changing the dictionary key to a string, which is the correct format for the RustExtension.

    10
    Best practice
    Specify a precise Python version range in requires-python to avoid including future, potentially unsupported versions

    The requires-python field should specify a range that includes only the supported
    versions, not using the tilde approximation which might unintentionally include
    future, unsupported Python versions.

    py/pyproject.toml [11]

    -requires-python = "~=3.8"
    +requires-python = ">=3.8, <3.13"
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion improves the precision of the Python version range, ensuring compatibility and preventing future unsupported versions from being included.

    8
    Adjust the version constraint for urllib3 to allow for minor updates within the same major version

    The dependency version for urllib3 should allow for minor updates within a major
    version, rather than restricting to a minor version range.

    py/pyproject.toml [29]

    -"urllib3[socks]>=1.26,<3"
    +"urllib3[socks]>=1.26,<2"
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Allowing minor updates within the same major version for urllib3 can improve compatibility and security by including minor updates and patches.

    7
    Enhancement
    Enable namespace packaging if the packages are part of a namespace

    The namespace field in the [tool.setuptools.packages.find] section should be set to
    true if the packages are intended to be part of a namespace package.

    py/pyproject.toml [40]

    -namespace = false
    +namespace = true
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: This suggestion is contextually accurate but should be verified if the packages are indeed part of a namespace. If not, this change could introduce issues.

    6

    @sandeepsuryaprasad
    Copy link
    Contributor Author

    @AutomatedTester could you review this PR please. Thanks!

    @VietND96 VietND96 added the C-py label Nov 1, 2024
    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