Skip to content
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

[chore] Fix update-otel #37706

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sincejune
Copy link
Contributor

Description

This PR fixed make update-otel to update builder-config.yaml correctly. This is the workflow we update builder-config.yaml files:

  1. MULTIMOD command to update go.mod
  2. updatehelper function to update builder-config.yaml according to the updated go.mod

However, make otelcontribcol/make oteltestbedcol will overwrite the updated go.mod unexpectedly before step 2.
This PR removes the unexpected make otelcontribcol/make oteltestbedcol commands.

cc @mx-psi

Link to tracking issue

n/a

Testing

n/a

Documentation

n/a

@sincejune sincejune requested a review from a team as a code owner February 5, 2025 13:39
@sincejune sincejune requested a review from ChrsMark February 5, 2025 13:39
Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

@songy23 songy23 added the ci-cd CI, CD, testing, build issues label Feb 5, 2025
Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

Please don't see the reason why was done the way was done.

However, make otelcontribcol/make oteltestbedcol will overwrite the updated go.mod unexpectedly before step 2.
This PR removes the unexpected make otelcontribcol/make oteltestbedcol commands.

Without this the go.mod is not available.

@mx-psi
Copy link
Member

mx-psi commented Feb 5, 2025

@bogdandrutu Can we fix the workflow first and then look into this issue?

@bogdandrutu
Copy link
Member

@bogdandrutu Can we fix the workflow first and then look into this issue?

Without this I cannot run the make update-otel, I am confused.

@bogdandrutu
Copy link
Member

Let me explain one more time:
In https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/cmd/otelcontribcol you don't have a go.mod, but the updatehelper requires it. So I am not sure how you intend to make this work.

@bogdandrutu bogdandrutu self-requested a review February 5, 2025 18:12
@bogdandrutu bogdandrutu dismissed their stale review February 5, 2025 18:12

I made my point, can move forward if you think this is the right thing

@sincejune
Copy link
Contributor Author

Please don't see the reason why was done the way was done.

However, make otelcontribcol/make oteltestbedcol will overwrite the updated go.mod unexpectedly before step 2.
This PR removes the unexpected make otelcontribcol/make oteltestbedcol commands.

Without this the go.mod is not available.

@bogdandrutu Thanks! Updated the PR according to your feedback. cc @mx-psi @codeboten

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-cd CI, CD, testing, build issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants