Skip to content

[release-1.4] pin go#1454

Merged
rhatdan merged 2 commits intocontainers:release-1.4from
mtrmac:1.4-pin-go
Sep 20, 2021
Merged

[release-1.4] pin go#1454
rhatdan merged 2 commits intocontainers:release-1.4from
mtrmac:1.4-pin-go

Conversation

@mtrmac
Copy link
Contributor

@mtrmac mtrmac commented Sep 20, 2021

1.17 has changed the expected gofmt format, and we don't want to follow such changes on the stable branch.

Then re-enable the osx task disabled in #1449.

An alternative to #1446 .

This reverts commit cd09650.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@lsm5 lsm5 requested a review from cevich September 20, 2021 13:42
1.17 has changed the expected gofmt format, and we don't want
to follow such changes on the stable branch.

go@1.16 is "keg-only", i.e. not installed by Brew to /usr/local/bin,
so we need to change PATH to point at it (as the installation instructs
us to).

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mtrmac
Copy link
Contributor Author

mtrmac commented Sep 20, 2021

@cevich PTAL, compare with current state post-#1449 , and #1446.

I’ll defer entirely to your judgment as to which approach is the best; keeping the current state with osx disabled would be perfectly fine with me.

@cevich
Copy link
Member

cevich commented Sep 20, 2021

keeping the current state with osx disabled would be perfectly fine with me.

I'm leaning this way, only from the perspective that few-jobs == less fail chance (the osx job also flakes due to homebrew repo. and/or networking problems). Just to confirm, am I correct believing these release branches see very little (if any) changes over time?

@mtrmac
Copy link
Contributor Author

mtrmac commented Sep 20, 2021

Just to confirm, am I correct believing these release branches see very little (if any) changes over time?

Historically that’s been very much the case. Right now the release-1.4 activity feels a little higher than usual to me, but that’s been mostly build-system changes, not features/security fixes driving the purpose of the branch. So I do still expect to see very few, if any, changes, in the future.

@rhatdan rhatdan merged commit fec1329 into containers:release-1.4 Sep 20, 2021
@cevich
Copy link
Member

cevich commented Sep 20, 2021

Oh I was going to say let's merge the other one, oh well, not a biggie. We can revert-revert the osx disablement later if needed 😀

@mtrmac mtrmac deleted the 1.4-pin-go branch September 22, 2021 20:40
@mtrmac
Copy link
Contributor Author

mtrmac commented Sep 22, 2021

@cevich So just to confirm, would you like the release-1.4 branch to remain as is, and #1446 to be dropped? Or to revert this and return to just disabling the test? Something else, like make the test pass using one or both methods, but disable it anyway?

I don’t really have a preference, I’m just looking for a way to have this matter decided, for now at least.

@cevich
Copy link
Member

cevich commented Sep 23, 2021

The PR was merged, so I became comfortable just ignoring it, now you've gone and ruined all my fun! 😄

Yes I think eventually (months/years) we'll probably end up disabling the OSX test again. If in the meanwhile #1446 happens, it's fine / won't hurt anything, and may help avoid a flake or three. I wouldn't suggest spending any more than 5-minutes on it though.

@cevich
Copy link
Member

cevich commented Sep 23, 2021

Clarification: meaning apply #1446 and leave the test enabled for now.

@mtrmac
Copy link
Contributor Author

mtrmac commented Sep 23, 2021

It’s 3 clicks to merge it, so I did that. Thanks!

@cevich
Copy link
Member

cevich commented Sep 23, 2021

And thank you 😃

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants