Skip to content

Over-haul of CI #166

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

Merged
merged 9 commits into from
Apr 9, 2025
Merged

Over-haul of CI #166

merged 9 commits into from
Apr 9, 2025

Conversation

surister
Copy link
Member

@surister surister commented Apr 8, 2025

For a long time I was unhappy with how the setup of this project ended up being, it was prone to breaking and made maintenance annoying. I've simplified and improve things to address these issues:

Summary of the changes / Why this is an improvement

  • Remove CrateDB service from testing CI
  • Update Python versions from 3.8, 3.13 to ["3.10", "3.11", "3.12", "3.13"]
  • Use uv in project setup, CI and cratedb-sqlparse python parser
  • Run only tests when library changes fixes Run github actions depending on path #39
  • Update cratedb_sqlparse_py pyproject's metadata
  • Update CrateDB version to 5.10.4
  • Create new release pipeline for NPM package. fixes Build JS target release pipeline. #115
  • Unify release in a single workflow release.yml and make it trigger able by GitHub releases.
  • Update both README and DEVELOP documentation

Checklist

surister added 5 commits April 8, 2025 14:39
The objective of the project is to be able to do SQL stuff without a CrateDB instance, so it does not make any sense to have one.
Removed 3.8 which reached EOL and ignored 3.9 which reaches it soon.

https://devguide.python.org/versions/
* Use uv in pipelines
* Add path for more fine-controlled executions fixes crate#39
* Change parent project to uv
* Update metadata
@surister surister changed the title Over haul of CI Over-haul of CI Apr 8, 2025
@surister surister requested review from amotl and kneth and removed request for amotl April 8, 2025 16:58
@surister surister marked this pull request as ready for review April 8, 2025 16:58
Copy link
Member

@amotl amotl left a comment

Choose a reason for hiding this comment

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

Excellent! 💯

Comment on lines +6 to +9
release_javascript:
uses: ./.github/workflows/release_javascript.yml
release_python:
uses: ./.github/workflows/release_python.yml
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

Comment on lines -90 to +96
urls.changelog = "https://github.com/crate/cratedb-sqlparse/blob/main/CHANGES.md"
urls.documentation = "https://github.com/crate/cratedb-sqlparse"
urls.homepage = "https://github.com/crate/cratedb-sqlparse"
urls.repository = "https://github.com/crate/cratedb-sqlparse"

[project.urls]
Homepage = "https://github.com/crate/cratedb-sqlparse"
Documentation = "https://github.com/crate/cratedb-sqlparse"
Repository = "https://github.com/crate/cratedb-sqlparse"
Changelog = "https://github.com/crate/cratedb-sqlparse/releases"
Copy link
Member

@amotl amotl Apr 8, 2025

Choose a reason for hiding this comment

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

Is this section compatible with the convention pyproject-fmt proposes?
poe format doesn't do it differently?

Copy link
Member Author

@surister surister Apr 9, 2025

Choose a reason for hiding this comment

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

I'd say so, I was actually getting errors when testing/building in python 3.14 if I recall and changing it this way fixed it, this is afaik the pep621 way of doing it, https://packaging.python.org/en/latest/specifications/well-known-project-urls/ so if pyproject-fmt complained (by not following the spec) I'd ditch it

Copy link
Member

@amotl amotl Apr 9, 2025

Choose a reason for hiding this comment

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

pyproject-fmt is not complaining. It just applies formatting conventions and is not violating PEP621 at all, it just applies a different formatting. I'd not ditch it, because it provides a canonical form for the project metadata file, with the same benefit for collaborative development like the other code formatters are providing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

it was a conditional statement

Copy link
Member

Choose a reason for hiding this comment

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

Hi again. This patch explains better what I was referring to.

Comment on lines +25 to +28
python -m pip install build twine
cd cratedb_sqlparse_py
python -m build
twine check dist/{*.tar.gz,*.whl}
Copy link
Member

@amotl amotl Apr 8, 2025

Choose a reason for hiding this comment

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

If you like it, a variant of those incantations could be:

cd cratedb_sqlparse_py
python -m pip install '.[release]'
poe release

release = [
"build<2",
"twine<7",
]

release = [
{ cmd = "python -m build" },
{ cmd = "twine upload dist/*" },
]

In this spirit, CI and manual procedures will be synchronized, through the canonical poe release command.

cratedb-sqlparse/DEVELOP.md

Lines 109 to 113 in 6542f48

#### Manual release.
Optionally, build the package and upload to PyPI manually.
```shell
uv run poe release
```

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.

Build JS target release pipeline. Run github actions depending on path
2 participants