Skip to content

feat: [Rust] UserDictからIndexMapinto可能にする #978

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

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

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Feb 7, 2025

内容

#977 の続き。一貫性のため。

関連 Issue

その他

これを機にUserDict::with_wordsというやつを封印してもいいかもしれません。
(それについての議論込みで今PRを出した次第です)

@qryxip qryxip requested a review from Hiroshiba February 7, 2025 04:01
@qryxip qryxip force-pushed the pr/feat-rust-impl-from-userdict-for-indexmap branch from aed84d7 to 33bf8fa Compare February 7, 2025 04:02
@qryxip qryxip force-pushed the pr/feat-rust-impl-from-userdict-for-indexmap branch from 33bf8fa to e35d98e Compare February 7, 2025 04:07
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.

うーーーーーーーーすみません!!!!
Rust APIの正式リリースは次バージョンで良い(リリースされていないものとして扱う)はず・・・・・・・!

なんとかマージしてあげたい気持ちはあるのですが、全部完成したあとで!!!!!!!!!!!!!!!!

あとどっちかというとUserDictはMap<words>にしない方向に倒したほうが将来的に良い気もちょっとします。

とりあえず揃えておくのは賛成なので、0.16ができたらすぐマージして0.16.1に追加とかもできるかも。
あるいは0.16のプレビュー版を公開したあと、正式リリースまでに変更するとかでもありかも。

@Hiroshiba Hiroshiba requested a review from Copilot March 8, 2025 20:07
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.

(ちょっと状況よくわからなくなったのでChatGPTにまとめてもらったので メモ 📝 )


これは #977 で変更された点ですが、UserDictをHashMap/IndexMapに変換可能にするのやめるのどうでしょう?
将来例えばUserDict自体の名前や著者やライセンスを書けるようにした場合、into()したときの値やto_dictの返り値が自明じゃなくなるので、関数を消すことになると思います。
まあそうなったときに破壊的変更すれば良いのですが、だったら最初(0.16)から.wordsで取得できる形にするのがベストなのでは、と・・・!

これを機にUserDict::with_wordsというやつを封印してもいいかもしれません。

これはどちらにせよ賛成です!

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.

ミス

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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