Skip to content

.github: Skip OpenTelemetry module in go 1.20 tests #7246

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

Closed
wants to merge 1 commit into from

Conversation

zasweq
Copy link
Contributor

@zasweq zasweq commented May 20, 2024

RELEASE NOTES: N/A

@zasweq zasweq requested a review from dfawley May 20, 2024 20:56
@zasweq zasweq added the Type: Meta Github repo, process, etc label May 20, 2024
@zasweq zasweq added this to the 1.65 Release milestone May 20, 2024
@zasweq zasweq force-pushed the ignore-opentelemetry-1.20 branch 2 times, most recently from 695634b to fe62af2 Compare May 20, 2024 21:20
@zasweq zasweq force-pushed the ignore-opentelemetry-1.20 branch from fe62af2 to 0ee4b76 Compare May 20, 2024 21:26
@@ -105,7 +105,10 @@ jobs:
cd "${GITHUB_WORKSPACE}"
for MOD_FILE in $(find . -name 'go.mod' | grep -Ev '^\./go\.mod'); do
pushd "$(dirname ${MOD_FILE})"
go test ${{ matrix.testflags }} -cpu 1,4 -timeout 2m ./...
# Skip OpenTelemetry module if go 1.20, Go OpenTelemetry does not support 1.20.
if [[ matrix.goversion != 1.20 ]] || [[ dirname != ./stats/opentelemetry* ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Why the *?

Copy link
Member

Choose a reason for hiding this comment

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

Also, can we do something clever here to ensure the condition is met once?

Meaning when we drop support for go1.20, I want to make sure we know to delete this. We can do that by (pseudo-code):

for MOD_FILE:
	if goversion == 1.20 && dirname == otel:
		skipped = true
	else:
		go test
if !skipped:
	fail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I don't know why I added *, for some reason I wanted starts with even though this is only one module. I'm going to move this inline with changes in other PR, good point. Ok will add that.

@dfawley dfawley assigned zasweq and unassigned dfawley May 20, 2024
@zasweq
Copy link
Contributor Author

zasweq commented May 20, 2024

Closing this PR, decided to inline in #7205.

@zasweq zasweq closed this May 20, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Meta Github repo, process, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants