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] fix packaging #14823

Merged
merged 1 commit into from
Nov 29, 2024
Merged

Conversation

benoit-pierre
Copy link
Contributor

@benoit-pierre benoit-pierre commented Nov 28, 2024

User description

Description

Ensure selenium.webdriver.common.fedcm gets packaged in.

# Before:
▹ python setup.py -q sdist && tar tf dist/selenium-4.28.0.202411261607.tar.gz | grep fedcm
selenium-4.28.0.202411261607/selenium/webdriver/remote/fedcm.py

# After:
▹ python setup.py -q sdist && tar tf dist/selenium-4.28.0.202411261607.tar.gz | grep fedcm
selenium-4.28.0.202411261607/selenium/webdriver/common/fedcm/
selenium-4.28.0.202411261607/selenium/webdriver/common/fedcm/__init__.py
selenium-4.28.0.202411261607/selenium/webdriver/common/fedcm/account.py
selenium-4.28.0.202411261607/selenium/webdriver/common/fedcm/dialog.py
selenium-4.28.0.202411261607/selenium/webdriver/remote/fedcm.py

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

Bug fix


Description

  • Ensure selenium.webdriver.common.fedcm is included in the package distribution.
  • Fix packaging issue to include missing fedcm module files.

Changes walkthrough 📝

Relevant files

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

@CLAassistant
Copy link

CLAassistant commented Nov 28, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

@benoit-pierre benoit-pierre force-pushed the pr/fix_python_packaging branch from 12be491 to 8242178 Compare November 28, 2024 00:19
@benoit-pierre benoit-pierre changed the title [pi] fix packaging [py] fix packaging Nov 28, 2024
Copy link
Contributor

@navin772 navin772 left a comment

Choose a reason for hiding this comment

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

Thanks @benoit-pierre for adding __init__.py to fedcm.
LGTM!

@navin772
Copy link
Contributor

@benoit-pierre can you also add the License headers to the file, it is required

Ensure `selenium.webdriver.common.fedcm` gets packaged in.
@benoit-pierre benoit-pierre force-pushed the pr/fix_python_packaging branch from 8242178 to 0e5bc8e Compare November 28, 2024 12:21
@VietND96
Copy link
Member

@navin772, do you have any idea on how to detect this issue via test, or CI when a new package would be added in future?

@mgorny
Copy link

mgorny commented Nov 28, 2024

You could run tests on installed package, i.e. in Gentoo we do the equivalent of:

pip install .
rm -rf selenium
pytest

@VietND96
Copy link
Member

Thanks for your suggestion!

@VietND96 VietND96 merged commit 5c49a66 into SeleniumHQ:trunk Nov 29, 2024
16 of 17 checks passed
@benoit-pierre benoit-pierre deleted the pr/fix_python_packaging branch November 29, 2024 10:18
@navin772
Copy link
Contributor

navin772 commented Dec 2, 2024

@VietND96 we can add a pre-check in python build CI that checks recursively inside py/selenium/ directory if __init__.py file exists in every dir.

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

Successfully merging this pull request may close these issues.

5 participants