search

stlab / libraries

ASL libraries will be migrated here in the stlab namespace, new libraries will be created here.
https://stlab.cc
Boost Software License 1.0
661 stars 65 forks source link

Fix CI #470

Closed dabrahams closed 2 years ago

dabrahams commented 2 years ago

Also makes our claims of testing with Apple clang not-a-lie.

dabrahams commented 2 years ago

Fixes #467

dabrahams commented 2 years ago

Could you also please update

https://github.com/stlab/libraries/blob/75418397bbedf7a1b11ce5ce05d1f670945e7a22/.github/matrix.json#L16

with whatever version number we're using now?

IMO that should happen after #472; otherwise we're just guessing.

dabrahams commented 2 years ago

@camio Please note also #471 Isn't it more important to get CI working again, so we can start shoring up this mare's nest properly?

camio commented 2 years ago

I'm assuming that the specification I linked to is correct. I have no reason to believe otherwise.

Also Clang/LLVM 13.1.6 is available, but using the macOS 12.4 image as specified here.

I'd rather a new inconsistency not be introduced in a PR. IMO we generally shouldn't knowingly introduce tech debt in a PR.

dabrahams commented 2 years ago

I'd rather a new inconsistency not be introduced in a PR. IMO we generally shouldn't knowingly introduce tech debt in a PR.

IMO CI being broken is an emergency, and this is a priority inversion. With the change it is no more inconsistent than it was. It said "apple clang 13.0.1" but that's not what we were testing; it was “stock clang 13.0.1”. This PR is clearly forward progress, and PR review shouldn't stand in the way of that. Sorry, I'm making the call and merging. See #473.

dabrahams commented 2 years ago

IMO that should be done after #472. OK?

On Jul 15, 2022, at 9:36 AM, David Sankel @.***> wrote:

@camio requested changes on this pull request.

Looks good. Could you also please update https://github.com/stlab/libraries/blob/75418397bbedf7a1b11ce5ce05d1f670945e7a22/.github/matrix.json#L16 https://github.com/stlab/libraries/blob/75418397bbedf7a1b11ce5ce05d1f670945e7a22/.github/matrix.json#L16 with whatever version number we're using now? Based on this https://github.com/actions/virtual-environments/blob/main/images/macos/macos-11-Readme.md it looks like this results in a switch to 13.0.0.

— Reply to this email directly, view it on GitHub https://github.com/stlab/libraries/pull/470#pullrequestreview-1040541728, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKYILHDSKP25XLUYSXNVTVUGHR5ANCNFSM53VTKPVA. You are receiving this because you authored the thread.

camio commented 2 years ago

I don't mind. In my mind, this is old tech debt and gets lumped in with all the rest as far as priority goes.

On Tue, Oct 11, 2022 at 3:02 AM Dave Abrahams @.***> wrote:

IMO that should be done after #472. OK?

On Jul 15, 2022, at 9:36 AM, David Sankel @.***> wrote:

@camio requested changes on this pull request.

Looks good. Could you also please update https://github.com/stlab/libraries/blob/75418397bbedf7a1b11ce5ce05d1f670945e7a22/.github/matrix.json#L16 < https://github.com/stlab/libraries/blob/75418397bbedf7a1b11ce5ce05d1f670945e7a22/.github/matrix.json#L16> with whatever version number we're using now? Based on this < https://github.com/actions/virtual-environments/blob/main/images/macos/macos-11-Readme.md> it looks like this results in a switch to 13.0.0.

— Reply to this email directly, view it on GitHub < https://github.com/stlab/libraries/pull/470#pullrequestreview-1040541728>, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAAKYILHDSKP25XLUYSXNVTVUGHR5ANCNFSM53VTKPVA . You are receiving this because you authored the thread.

— Reply to this email directly, view it on GitHub https://github.com/stlab/libraries/pull/470#issuecomment-1274180167, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4YR24PRP3PWJKBJ2K4IBDWCUGKHANCNFSM53VTKPVA . You are receiving this because you were mentioned.Message ID: @.***>