[py] Getting rid of `MANIFEST.in` file by moving non-default file info to `pyproject.toml`

sandeepsuryaprasad commented 3 months ago

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

In this PR I have moved all the information about the non-default file information that gets shipped as part of selenium package from MANIFEST.in to pyproject.toml file.

Motivation and Context

This eliminates the need of having a separate config file.
Also, recursive discovery of all packages can be specified in pyproject.toml file through table [tool.setuptools.packages.find] , the package information in setup.py can be eliminated and the same can be maintained in pyproject.toml.

Types of changes

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

Checklist

[x] 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.
[x] All new and existing tests passed.

PR Type

Enhancement, Configuration changes

Description

Removed MANIFEST.in file and moved its contents to pyproject.toml.
Updated setup.py to eliminate package configuration, now handled by pyproject.toml.
Modified Bazel build configuration to remove reference to MANIFEST.in.

Changes walkthrough 📝

Relevant files

Configuration changes

setup.py `Remove package configuration from setup.py` py/setup.py Removed `package_dir` and `packages` definitions. Eliminated `include_package_data` setting.	+0/-21
BUILD.bazel `Update Bazel build configuration` py/BUILD.bazel - Removed reference to `MANIFEST.in` file.	+0/-1
MANIFEST.in `Remove MANIFEST.in file` py/MANIFEST.in - Deleted the `MANIFEST.in` file entirely.	+0/-24
pyproject.toml `Add package and data configuration to pyproject.toml` py/pyproject.toml Added package discovery and data inclusion settings. Included non-default file information.	+19/-0

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

codiumai-pr-agent-pro[bot] commented 3 months ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

Configuration Error
The `selenium_package` key in `[tool.setuptools.package-data]` might not be recognized by setuptools. Usually, the key should match the package name exactly, which is typically 'selenium' rather than 'selenium_package'. Missing Package Definitions
The removal of 'package_dir' and 'packages' from setup.py without ensuring they are correctly handled in pyproject.toml could lead to issues in package discovery and installation.

codiumai-pr-agent-pro[bot] commented 3 months ago

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Correct the file pattern to ensure it matches the intended files ___ *Ensure that the `selenium.egg-info ` pattern correctly matches intended files. It appears to have a space which might cause it to not work as expected.** [py/pyproject.toml [17]](https://github.com/SeleniumHQ/selenium/pull/14309/files#diff-3adb340b5a499e26b04507d972c31f5cf6fc27a6d81e3f7b547bf50e90c43be3R17-R17) ```diff -"selenium.egg-info ", +"selenium.egg-info/", ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 9 Why: Correcting the file pattern is crucial to ensure that the intended files are matched correctly, preventing potential build issues.	9
Maintainability	Use more specific file patterns to avoid including unwanted files ___ It's recommended to use a more specific glob pattern or explicitly list the files in the `selenium_package` to avoid unintentionally including unwanted files. [py/pyproject.toml [11-21]](https://github.com/SeleniumHQ/selenium/pull/14309/files#diff-3adb340b5a499e26b04507d972c31f5cf6fc27a6d81e3f7b547bf50e90c43be3R11-R21) ```diff selenium_package = [ ".py", ".rst", ".json", ".xpi", ".js", - "selenium.egg-info ", + "selenium.egg-info/", "selenium-manager", "selenium-manager.exe", "CHANGES", "LICENSE" ] ``` - [ ] Apply this suggestion* Suggestion importance[1-10]: 8 Why: Using more specific glob patterns helps to avoid unintentionally including unwanted files, improving maintainability and reducing potential issues.	8
Maintainability	Remove redundant default settings for cleaner code ___ The `namespaces` key in `[tool.setuptools.packages.find]` is set to `false`, which is the default value. Consider removing it for cleaner code unless you explicitly need to document this setting. [py/pyproject.toml [7]](https://github.com/SeleniumHQ/selenium/pull/14309/files#diff-3adb340b5a499e26b04507d972c31f5cf6fc27a6d81e3f7b547bf50e90c43be3R7-R7) ```diff -namespaces = false +# namespaces = false ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 6 Why: Removing redundant settings can make the code cleaner, but the impact on functionality is minimal since it is a default value.	6
Best practice	Specify dependency versions to ensure compatibility ___ Consider specifying the versions for the `install_requires` dependencies to ensure compatibility and prevent potential future conflicts. [py/setup.py [59-60]](https://github.com/SeleniumHQ/selenium/pull/14309/files#diff-722aeeec2821793eb6adf0c529cc7439c4b27ce78937cbb8840e94d4fc1c4017R59-R60) ```diff -"urllib3[socks]>=1.26,<3", -"trio~=0.17", +"urllib3[socks]>=1.26,<1.27", +"trio~=0.17.0", ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: Specifying exact versions for dependencies can help prevent compatibility issues, but the suggested versions may not be necessary if the current version constraints are sufficient.	7

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 59.12%. Comparing base (215e20b) to head (523c2e3). Report is 4 commits behind head on trunk.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## trunk #14309 +/- ## ========================================== + Coverage 59.09% 59.12% +0.03% ========================================== Files 91 91 Lines 5830 5835 +5 Branches 260 260 ========================================== + Hits 3445 3450 +5 Misses 2125 2125 Partials 260 260 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sandeepsuryaprasad commented 3 weeks ago

@VietND96 can you please re-trigger the workflow for this PR.. I have updated the branch.. Thanks!

SeleniumHQ / selenium