-
Notifications
You must be signed in to change notification settings - Fork 3
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
Node.js v22 対応 #1057
base: master
Are you sure you want to change the base?
Node.js v22 対応 #1057
Changes from 5 commits
7f16ebf
bbb64c7
cc65303
f4a7dd9
d4c3cc4
2c68407
6443d2e
b9225d7
ac085d2
aa24f98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,4 +53,4 @@ MIT | |
|
||
## funding | ||
|
||
永続的な開発を続けるために、寄付を歓迎します。 | ||
永続的な開発を続けるために、寄付を歓迎します。 |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -235,6 +235,9 @@ function countTimeRange( | |||||||||
log.endMs < range.endMs && | ||||||||||
log.endMs - range.startMs > ACTIVITY_COUNT_INTERVAL_THRESHOLD_MS) | ||||||||||
) { | ||||||||||
if (range.count === undefined) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Node.js v22関係なくおそらくlintが落ちていたようなので、修正しました(他のCIが通ることを検証したかったため)。 |
||||||||||
range.count = 0; | ||||||||||
} | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 一瞬だけ検討してみるくらいでOKですがこのように短く書いても読みやすい(個人差ありなので任意)かも
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 修正しました! |
||||||||||
range.count += 1; | ||||||||||
} | ||||||||||
}); | ||||||||||
|
@@ -350,7 +353,10 @@ async function upsertActivity({ | |||||||||
} | ||||||||||
|
||||||||||
const timeRanges = merge(exists?.timeRanges ?? [], activity.timeRanges); | ||||||||||
const timeRangeLogs = concatAndMerge(recentTimeRangeLogs, activity.timeRanges); | ||||||||||
const timeRangeLogs = concatAndMerge( | ||||||||||
recentTimeRangeLogs, | ||||||||||
activity.timeRanges | ||||||||||
); | ||||||||||
const purgedTimeRangeLogs = purge(recentTimeRangeLogs, activity.timeRanges); | ||||||||||
|
||||||||||
timeRangeCounts = countTimeRange(timeRangeCounts, purgedTimeRangeLogs); | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 差分はformatの改行のみです。 |
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.
LTSの最新のバージョンに固定するように修正しました(ローカル環境のNode.jsとバージョンを固定化するため)が、問題あればコメントください。
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.
ローカル環境のNode.jsとバージョンが違うことで問題を生む可能性がありそうですね
そういう点で固定化するのは良いと思います 👍
あるいはこういうとき別の対処として package.json あるいは .nvmrc や .node-version-file を指定してワークツリーの目立つところのに配置して他のツールのサポート(e.g. nvm、mise)を受けられるようにしてみるのもいいかも
一度ご検討くださいませ
具体例:
https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#node-version-file
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.
ファイル指定できること知見でした!ありがとうございます。package.jsonを参照する形に修正しました