-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat(log): move finished log #27
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pull request ありがとうございます!
挙動が変更となる部分についてコメントしました🙏
@@ -44,6 +41,7 @@ public struct Fetcher { | |||
for try await (repository, releases) in group { | |||
results[repository] = releases | |||
} | |||
Logger.shared.info("🎉 \(#function) finished") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変更により、Fetcher.fetch(repositories:)
がエラーだったときに何も log が残らないため、エラー時もその旨の log を追加したいです。
(1つ上にある Fetcher.fetch(repository:)
にもエラー時の log を追加したいです。)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
スローした先でログを出力するので、ここでは何も残さなくていいと思うんだけどどうだろう?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ほかも同様
@@ -71,13 +68,11 @@ public struct FileHelper { | |||
Logger.shared.info("🔔 \(repository.outputJSONFileName) loading was skipped because that file could not be found") | |||
} | |||
} | |||
Logger.shared.info("🎉 \(#function) finished") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変更により、FileHelper.load(repositories:)
がエラーだったときに何も log が残らないため、エラー時もその旨の log を追加したいです。
@@ -86,17 +81,14 @@ public struct FileHelper { | |||
try data.write(to: url) | |||
Logger.shared.info("✅ Saved \(repository.outputJSONFileName)") | |||
} | |||
Logger.shared.info("🎉 \(#function) finished") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変更により、FileHelper.save(contents:)
がエラーだったときに何も log が残らないため、エラー時もその旨の log を追加したいです。
@@ -112,19 +104,18 @@ public struct FileHelper { | |||
+ upperBoundKeyword | |||
try string.replacingCharacters(in: lowerBound..<upperBound, with: outputListOfRepositoriesString) | |||
.write(to: url, atomically: true, encoding: .utf8) | |||
Logger.shared.info("🎉 \(#function) finished") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変更により、FileHelper.writeToREADME(repositories:)
がエラーだったときに何も log が残らないため、エラー時もその旨の log を追加したいです。
Logger.shared.info("ℹ️ \(#function) started") | ||
let url = URL.topLevelDirectory.appendingPathComponent("README.md") | ||
let string = try String(contentsOf: url) | ||
guard let lowerBound = string.range(of: lowerBoundKeyword)?.lowerBound, | ||
let upperBound = string.range(of: upperBoundKeyword)?.upperBound else { | ||
throw Error.invalidREADMEFormat | ||
} | ||
Logger.shared.info("🎉 \(#function) finished") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変更により、FileHelper.readFromREADME()
がエラーだったときに何も log が残らないため、エラー時もその旨の log を追加したいです。
Logger.shared.info("🎉 \(#function) finished") | ||
return .releases(destination, .init(name: name, owner: owner, repository: repository)) | ||
case "tags": | ||
Logger.shared.info("✅ The correct YAML format: \(name) (\(`case`))") | ||
Logger.shared.info("🎉 \(#function) finished") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変更により、Parser.parse()
がエラーだったときに何も log が残らないため、エラー時もその旨の log を追加したいです。
@@ -60,6 +57,7 @@ public struct SlackNotifier { | |||
} | |||
try await group.waitForAll() | |||
} | |||
Logger.shared.info("🎉 \(#function) finished") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変更により、SlackNotifier.notify(to:updates:)
がエラーだったときに何も log が残らないため、エラー時もその旨の log を追加したいです。
こちらの部分は意図的で、メソッドが成功・失敗によらず終了したか?を出力したかった(ので 確実にスコープを抜けたときに実行される defer を用いていた)という経緯でした。 |
意図は理解できてる! |
完了のログが出力されていない = 途中で処理が中断されたんだな、とわかる |
わかります。そのため以下のように error としての log を残したいです。
|
defer
でログを出力すると、スロー時にも実行されてどこで処理が失敗したかわかりづらいので、各関数の最後に呼ぶようにした。