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

Configuring with pyproject.toml #996

Merged

Conversation

WilliamBergamin
Copy link
Contributor

These changes started out as an exploration of the pyproject.toml file. In the process I learnt that since PEP 621, the Python community selected pyproject.toml as a standard way of specifying project metadata.

This PR aims to introduce changes that make the pyproject.toml the default configuration file for bolt-python, there are some slight differences but the testing, packaging and publishing behaviors should remain the same as they were

I've opted to rely on the Requirements File Format pattern to import test dependencies since it yields a clear distinction between the project dependencies and the development dependencies

Is the pyproject.toml configuration pattern something we would want to adopt in our python projects?

Category

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter
  • Document pages under /docs
  • Others

Requirements

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run ./scripts/install_all_and_run_tests.sh after making the changes.

@seratch
Copy link
Member

seratch commented Nov 28, 2023

@WilliamBergamin Thanks for proposing this! As long as the release process goes well, migrating to pyproject.toml should be a good way to go! Can you adjust the CI build settings to pass the tests? Also, checking if the package upload works using https://test.pypi.org/ would be safe.

@@ -28,10 +28,10 @@ jobs:
run: |
python setup.py install
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps, need changes on this part

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point, I've fixed up the CI parts to work with the new changes

NOTE: there are some limitation with python 3.6 that required work arounds

Copy link

codecov bot commented Nov 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9209e7b) 91.76% compared to head (76c668a) 91.76%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #996   +/-   ##
=======================================
  Coverage   91.76%   91.76%           
=======================================
  Files         181      181           
  Lines        6312     6312           
=======================================
  Hits         5792     5792           
  Misses        520      520           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@WilliamBergamin
Copy link
Contributor Author

@seratch I've deployed a test package to https://test.pypi.org/project/slack-bolt/ and everything seems to be the same apart from the "Homepage" link being renamed to "homepage"

Could you take a look and let me know if you find any discrepancies

@WilliamBergamin WilliamBergamin marked this pull request as ready for review November 28, 2023 17:21
@seratch
Copy link
Member

seratch commented Nov 28, 2023

Thank you! I just confirmed the package file uploaded to test PyPI works without any issues.

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

One nit comment but this looks great to me! After fixing the minor error in a comment, you can merge this PR.

requirements/adapter.txt Outdated Show resolved Hide resolved
@seratch seratch added this to the 1.18.2 milestone Nov 28, 2023
@seratch
Copy link
Member

seratch commented Nov 28, 2023

Also, when you merge this, please use "Squash and merge" button to make the diffs simpler

@WilliamBergamin WilliamBergamin merged commit 90467fb into slackapi:main Nov 29, 2023
12 checks passed
@WilliamBergamin WilliamBergamin deleted the consolidate-pyproject-toml branch November 29, 2023 19:06
@WilliamBergamin WilliamBergamin mentioned this pull request Dec 1, 2023
8 tasks
@seratch seratch modified the milestones: 1.18.2, 1.19.0 May 8, 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