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

build: Poetry v2を使う #920

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Jan 8, 2025

内容

CIが壊れているのを直すついでに、Poetryをv2に上げる。

See-also: https://python-poetry.org/blog/announcing-poetry-2.0.0/

関連 Issue

その他

@qryxip qryxip requested a review from Hiroshiba January 8, 2025 17:29
@qryxip qryxip force-pushed the build-use-poetry-v2 branch from a77b8e6 to 1962b45 Compare January 8, 2025 17:33
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM!

僕は正直、メジャーバージョンがリリースされた直後のライブラリを使うのはバグが埋め込まれている可能性が高いので結構反対ですが、まあでもクリティカルな問題にはならない気がするのと、あと問題があったとしても可及的速やかに解決されそうだと思い、特に問題ではなさそうだなと思いました!

@@ -225,7 +225,9 @@ jobs:
- name: set cargo version
run: |
cargo set-version "$VERSION" --exclude voicevox_core_python_api --exclude downloader --exclude xtask
if ${{ matrix.python_whl }}; then cargo set-version "$VERSION" -p voicevox_core_python_api; fi
if ${{ matrix.python_whl }}; then
sed -i_ 's/version = "\(0\.0\.0\)"/version = "'"$VERSION"'"/' ./crates/voicevox_core_python_api/pyproject.toml
Copy link
Member

Choose a reason for hiding this comment

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

(ただのコメントです)

sedって置き換え対象の文字列がなかったとしてもエラーにならないんですよねー。。。
例えば今回の場合はversion = "\(0\.0\.0\)"のとこが別の値に変わってバージョンが入らなくなっていてもビルド時にきづけない。

あとsedはmacOSだとBSD版?が入ってて扱いづらいという。

一応s/置換前の文字列/置換後の文字列/; t; q1;などとすれば、書き換え対象の文字列がなかった場合exit 1してくれるっぽみです。

あるいはビルド時にバージョンが一致しているかどうかを確かめてもいいかも?

まあ、参考になれば。。。

@qryxip qryxip merged commit 64f27d9 into VOICEVOX:main Jan 8, 2025
35 checks passed
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.

2 participants