Conversation
aherrmann
left a comment
aherrmann
left a comment
There was a problem hiding this comment.
Awesome, thank you! A couple small comments.
| # ------------------------------ | ||
| build:linux-nixpkgs --host_platform=@io_tweag_rules_nixpkgs//nixpkgs/platforms:host | ||
| build:macos-nixpkgs --host_platform=@io_tweag_rules_nixpkgs//nixpkgs/platforms:host | ||
| build:nixpkgs --host_platform=@io_tweag_rules_nixpkgs//nixpkgs/platforms:host |
There was a problem hiding this comment.
This breaks the config flags documented in the README. With this change the last reference to linux-nixpkgs is gone and --config=linux-nixpkgs becomes invalid. I'd prefer to keep the config matrix in the README valid for symmetry with the bindist cases and in case we need platform specific flags in the future.
There was a problem hiding this comment.
Bazel doesn't appear to support empty configurations (would make a good feature request upstream). We could hack around that, but I opted instead to remove this configuration from the README.
There was a problem hiding this comment.
I've opened such a feature request: bazelbuild/bazel#12844.
I don't think that change is correct. On Linux/Nixpkgs we would need --config=nixpkgs but the README claims (unneeded). On macOS we would now need both --config=nixpkgs --config=macos-nixpkgs but the README claims --config=macos-nixpkgs. The aim of #1301 was to require only one top-level --config flag for developers.
I've pushed a commit that loads --config=nixpkgs from both linux-nixpkgs and macos-nixpkgs. With this developers only need a single top-level config and both linux-nixpkgs and macos-nixpkgs unfold the nixpkgs config as well.
There was a problem hiding this comment.
I'm not sure I follow. --config=nixpkgs doesn't exist in this branch.
There was a problem hiding this comment.
Sorry, spurious comment. I had the wrong branch checked out.
| @@ -1,45 +0,0 @@ | |||
| steps: | |||
There was a problem hiding this comment.
tests/RunTests.hs now contains dangling references to .circleci and .buildkite in comments that need to be fixed.
aherrmann
force-pushed
the
gh-actions-all
branch
from
bac7408 to
47b83e0
Compare
|
@aherrmann just realized that yesterday when I did the force push, this somehow went to This is probably why I use Magit, which has a concept of "upstream" and "push" branch: https://magit.vc/manual/magit/The-Two-Remotes.html. This is likely culprit. I noticed before that it interacts badly with using Git CLI. |
Hmm, I thought branch protection on |
|
AFAIU branch protection only protects against force pushes from admins.
Though we can enable stronger protection.
|
I see, if I understand these docs correctly then admins can always push.
|
|
This was failing on buildifier warnings |
|
That looks like some potential network causing a timeout to be hit - the invocation uploaded just fine on our end, and I can't find any errors in the logs related to that invocation: Are you seeing this consistently? |
|
Thanks for looking into this @siggisim .
No, this was the first time I noticed this. Yes, looks like network flakiness. I'll let you know if this happens repeatedly. |
This reverts commit 6a7d835.
It now points you to the master branch history.
aherrmann
force-pushed
the
gh-actions-all
branch
from
f5249b6 to
3be1b15
Compare
Tested on #1471 in Mergify Config Editor UI.
|
I've included an update to the mergify configuration and checked it in their config editor UI. |
3 participants
Hot on the heals of #1460, we move the Linux jobs to GitHub Actions as
well. In practice, BuildKite with custom runners wasn't buying us that
much time, though we may return to custom runners (via Actions) later.
The only CI job not on Actions is now the Windows one.