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

全てのimportを同じ概念なら./、そうじゃないなら@/で書き換える #295

Open
y-chan opened this issue Oct 3, 2021 · 4 comments

Comments

@y-chan
Copy link
Member

y-chan commented Oct 3, 2021

内容

PRのレビューコメントを見て思ったのですが、コード全体を通してimport方法が結構バラバラになっているのが少々気になります。

https://github.com/Hiroshiba/voicevox/pull/291#r720796691

同じディレクトリ内でも@/./と2種類の表記が混じっているところもあり、統一性が見られません。必ずしも統一する必要はないと言われればそこまでなので、このIssueは閉じますが、そのファイルがどこに配置されているものなのかがパッと見てわかる@/で統一すると良いかもしれません。
また、その書き方以外を許容しないような設定をeslintに組み込むと以降の統一性が保てると考えます。

Pros 良くなる点

ただただimportの形式を統一するだけのリファクタですが、コードの可読性を向上させることが出来ると思います。

Cons 悪くなる点

ないと思われます

実現方法

importを書き換える

VOICEVOXのバージョン

0.6.1

その他

@Hiroshiba
Copy link
Member

Hiroshiba commented Oct 3, 2021

ディレクトリ構成を変えたときに変更が生じにくい方法が良いかなと思います。

例えば同じ概念なのであれば、./などの相対的なimportの方が良いと個人的には思います。
ディレクトリ構成を変えるときもきっと一緒に移動するためです。

別の概念をimportするときは@/が良さそうに感じます。
ディレクトリ構成を変えるときに離れる可能性が高そうだからです。

このリファクタリングは大歓迎です!

@y-chan
Copy link
Member Author

y-chan commented Oct 3, 2021

例えば同じ概念なのであれば、./などの相対的なimportの方が良いと個人的には思います。

なるほど、確かにこれは良いと思いました。
であれば、どちらかというと、同じ概念の中に含まれているもので@/の形になっているものを./の形に戻すことが多くなりそうですね(というかその形しか残ってない...?)。

@Hiroshiba
Copy link
Member

たしかに・・・。
まあでも今はどちらかというとコードの綺麗さよりも高速で実装していくフェーズで、まだ概念の切り分けが完了していていないため、後回しにしたほうがより綺麗に分けられるかなと思いました!

@Hiroshiba
Copy link
Member

こちらのタスクは「同じ親のディレクトリだった場合は相対インポート(./)、そうじゃなければ絶対インポート(@/)」というタイトルが合ってそうです。
ということでそのように変更し、中身も変えさせていただきます!

@Hiroshiba Hiroshiba changed the title 全てのimportを@/で書き換える 全てのimportを同じ概念なら./@/で書き換える Feb 25, 2024
@Hiroshiba Hiroshiba changed the title 全てのimportを同じ概念なら./@/で書き換える 全てのimportを同じ概念なら./、そうじゃないなら@/で書き換える Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants