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
Reenable caching for nix workflows #631
Reenable caching for nix workflows #631
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
9164bd6
to
00af085
Compare
00af085
to
8ee4ad9
Compare
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.
Reviewable status: 0 of 1 LGTMs obtained
.github/workflows/image.yaml
line 30 at r1 (raw file):
with: github-token: ${{ secrets.GITHUB_TOKEN }} # TODO(aaronmondal): Remove after:
I am seeing this comment a few places but this PR was recently merged. As a result, should we remove it? I wasn't sure what was meant by the comment. Should this section be removed since 820 was merged or does it get skipped after, which is now, since it was just merged?
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.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained
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.
Reviewed all commit messages.
Reviewable status: 1 of 1 LGTMs obtained
8ee4ad9
to
434d404
Compare
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.
Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained
The previous crashes have been fixed in Nix 2.19.3. Thix commit also includes missing `github-token` fields which increase the API rate limit. Fixes: TraceMachina#464
434d404
to
b33a8c5
Compare
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.
Reviewed 5 of 5 files at r3, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained
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.
Reviewable status: 2 of 1 LGTMs obtained
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.
Reviewable status: 2 of 1 LGTMs obtained
.github/workflows/image.yaml
line 30 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
I am seeing this comment a few places but this PR was recently merged. As a result, should we remove it? I wasn't sure what was meant by the comment. Should this section be removed since 820 was merged or does it get skipped after, which is now, since it was just merged?
Done.
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.
My bad. Added reviewers in the wrong way.
+@MarcusSorealheis +@adam-singer +@allada
Reviewable status: 2 of 3 LGTMs obtained (waiting on @adam-singer)
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.
Reviewable status: 2 of 2 LGTMs obtained
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.
Reviewable status: 1 of 1 LGTMs obtained
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.
Dismissed @MarcusSorealheis from a discussion.
Reviewable status: complete! 1 of 1 LGTMs obtained
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.
Reviewed all commit messages.
Reviewable status: complete! 1 of 1 LGTMs obtained
This has been fixed upstream. Pull the change in early to speed up
workflows significantly. This also includes missing
github-token
fields which increase the API rate limit.
Fixes: #464
Type of change
This change is