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

feat: migrate from setup.py/setuptools to pyproject.toml/hatch #99

Open
wants to merge 1 commit into
base: release
Choose a base branch
from

Conversation

Faraz32123
Copy link
Collaborator

@Faraz32123 Faraz32123 commented Jan 24, 2025

@regisb
Copy link
Contributor

regisb commented Jan 24, 2025

I trust your judgment and I'm removing myself from the reviewers of this PR.

@regisb regisb removed their request for review January 24, 2025 15:21
@Faraz32123 Faraz32123 force-pushed the feat/faraz_migrate_from_setup_py_and_setuptools_to_pyproject_toml_and_hatch branch 2 times, most recently from c383e30 to 527251d Compare January 24, 2025 15:26
@Faraz32123 Faraz32123 self-assigned this Jan 24, 2025
@Faraz32123 Faraz32123 marked this pull request as ready for review January 24, 2025 15:28
@@ -18,10 +18,8 @@ jobs:
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Upgrade pip
run: python -m pip install --upgrade pip setuptools
Copy link
Contributor

Choose a reason for hiding this comment

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

should we not keep pip in here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's just extra, as we are only upgrading pip, it's pre-compiled version is sufficient I think.
Plus, it's synchronized with the tutor core PR here.

@@ -0,0 +1,3 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: empty line at the start.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

missed it! removed.

[tool.hatch.build.targets.sdist]
# Disable strict naming, otherwise twine is not able to detect name/version
strict-naming = false
exclude = ["tests*"]
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference in egg_file now that we are only excluding tests*? The MANIFEST.in included only certain files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have now added include statement! may be we can restrict it to only templates and patches folder also!

- migrate from setup.py/setuptools to pyproject.toml/hatch.
- This commit will keep the tutor-discovery in sync with the tutor core. For more details view this PR in tutor: overhangio/tutor#1163.
@Faraz32123 Faraz32123 force-pushed the feat/faraz_migrate_from_setup_py_and_setuptools_to_pyproject_toml_and_hatch branch from 527251d to 5143b96 Compare January 27, 2025 16:14
- name: Install dependencies
run: |
run: pip install pylint black |
Copy link
Member

Choose a reason for hiding this comment

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

Previously, we had a dev environment, which installed tutor[dev] (and that, handled these dependencies). Why we're not including that in our pyproject.toml?
test job logs specifically says that
WARNING: tutor-discovery 19.0.0 does not provide the extra 'dev'

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

Successfully merging this pull request may close these issues.

4 participants