Skip to content
This repository has been archived by the owner on Apr 12, 2023. It is now read-only.

Refactor/remove unused classes #115

Merged
merged 7 commits into from
Jul 13, 2021
Merged

Refactor/remove unused classes #115

merged 7 commits into from
Jul 13, 2021

Conversation

keiji
Copy link
Collaborator

@keiji keiji commented Apr 13, 2021 •
edited
Loading

Issue 番号 / Issue ID

#114

目的 / Purpose

アプリから使用していないクラスやリソースを削除する

破壊的変更をもたらしますか / Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request の種類 / Pull Request type

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

検証方法 / How to test

コードの入手 / Get the code

gh pr checkout 115
dotnet restore

確認事項 / What to check

  • ビルドが問題なく完了すること
  • 利用規約・プライバシーポリシー・ライセンス一覧が表示されること

その他 / Other information

UnUsedディレクトリのクラスはもともとプロジェクトの対象から外れていたので、容量削減への影響はない。

Documentsに移動させたファイル群は現在は外部URLを閲覧するようになっていて、参照URLは文字列リソースで定義されている。

@keiji keiji mentioned this pull request Apr 13, 2021
Closed
6 tasks
@keiji keiji self-assigned this Apr 13, 2021
@keiji keiji marked this pull request as ready for review April 13, 2021 06:06
keiji
keiji commented Apr 13, 2021
View reviewed changes
Covid19Radar/Covid19Radar/Resources/html/en/chatbot2.html
@@ -1,87 +0,0 @@
<!DOCTYPE HTML>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

この辺のファイルはアプリから使われていないけどサーバー側にはあるので、Projectの外に移動させるのが適当

https://covid19radarjpnprod.z11.web.core.windows.net/en/chatbot2.html

Copy link
Contributor

Choose a reason for hiding this comment

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

Chatbot は 047b12b で削除されていて現在は使われていない気がします。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ありがとうごございます。
Chatbotは削除されましたが、chatbot2.html と同名のものがサーバー側にあるので、こちらはオリジナルとして削除はせず、licenseやtermofuseと同様にプロジェクトの外に移動させるにとどめるのが無難かなと思った次第です。

https://github.com/cocoa-mhlw/cocoa/blob/master/Covid19Radar/Covid19Radar/ViewModels/HomePage/ContactedNotifyPageViewModel.cs#L35

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

うーん。
プロジェクトにあるchatbot2.htmlとサーバー側にあるchatbot2.html、かなり構造が違いますね…

Copy link
Contributor

Choose a reason for hiding this comment

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

そもそもどうして Chatbot は削除されたのでしょうか？

Copy link
Contributor

Choose a reason for hiding this comment

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

@keiji
このHTMLファイルはすでに使われていませんので削除でお願いします。
初回リリース前にこれらのファイルをアプリ内で持っていた名残だと思います。

Copy link
Contributor

Choose a reason for hiding this comment

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

@keiji
上記コメントの件、ご確認いただけますか？

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cocoa-dev chatbot系のhtmlファイルと関連css, imgを削除しました。

Copy link
Contributor

Choose a reason for hiding this comment

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

@keiji
あ、ライセンスとか利用規約のファイルなどは残っていたんですね。見落としていました。すみません。
こちらはマージにされてしまいましたが、上記同様残しておく必要性が無いので使われていないので削除で良かったですね。

@keiji keiji requested a review from cocoa-dev May 18, 2021 00:52
@keiji keiji changed the base branch from master to develop June 7, 2021 07:17
@b-wind b-wind mentioned this pull request Jun 14, 2021
Merged
2 tasks
@Takym Takym mentioned this pull request Jun 14, 2021
Open
@keiji keiji mentioned this pull request Jun 15, 2021
Merged
@cocoa-dev
Copy link
Contributor

cocoa-dev commented Jul 9, 2021 •
edited
Loading

@keiji
Covid19Radar.csprojファイルに、UnUsedフォルダの参照が残っています。
Removed属性なので使われないのだとは思いますが残しておく必要もないと思うので可能でしたら削除お願いします。

@keiji
Copy link
Collaborator Author

keiji commented Jul 9, 2021

ご指摘ありがとうございます！　消します！

@cocoa-dev
Copy link
Contributor

@keiji
PRのもの(スクショの左)と、v1.2.4(スクショの右)のもので比較してみたのですが、なぜか見た目に違いが出てしまっています。
違いが出てしまうことは想定していないと思うのでご確認いただければと思います。
※スクリーンショットの画面以外でも違いがあります
※どちらも私の環境でのデバッグビルドです
※スクリーンショットはPixel 4aのものです
スクリーンショット 2021-07-09 14 55 04

@keiji
Copy link
Collaborator Author

keiji commented Jul 9, 2021

これはまだNotoCJKフォントがあるときのPullRequestですから、そのままチェックアウトすると以前（フォント適用時）の見た目で見えると思います。

マージすると見た目は1.2.4と一致するはずですが、この際、ついでに最新のdevelopブランチにrebaseしますね。

@cocoa-dev
Copy link
Contributor

これはまだNotoCJKフォントがあるときのPullRequestですから、そのままチェックアウトすると以前（フォント適用時）の見た目で見えると思います。

マージすると見た目は1.2.4と一致するはずですが、この際、ついでに最新のdevelopブランチにrebaseしますね。

承知しました。よろしくお願いいたします。

@keiji keiji force-pushed the refactor/remove_unused_classes branch from 9df1f5a to d743d0e Compare July 9, 2021 07:20
@keiji
Copy link
Collaborator Author

keiji commented Jul 9, 2021

rebased

@cocoa-dev
Copy link
Contributor

v1.2.4と同様のレイアウトになりました。ありがとうございます。

@keiji
Copy link
Collaborator Author

keiji commented Jul 9, 2021

@cocoa-dev
フォントの廃止で若干の見た目の変化があったことは把握していましたが、こうして並べてみるとフォント廃止前の方がバランスがいいですね……。徐々にでも調整できればと思うので、微調整のIssue立てます。

@keiji keiji merged commit 7821805 into cocoa-mhlw:develop Jul 13, 2021
@keiji keiji deleted the refactor/remove_unused_classes branch July 13, 2021 00:27
@keiji keiji mentioned this pull request Jul 13, 2021
Closed
@keiji keiji mentioned this pull request Jul 13, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Takym Takym Takym left review comments

@cocoa-dev cocoa-dev cocoa-dev approved these changes

Assignees

@keiji keiji

@cocoa-dev cocoa-dev

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@keiji @cocoa-dev @Takym