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

Update (or create) opentelemetrybot/semantic-conventions-v* branch daily #6081

Merged
merged 7 commits into from
Jan 30, 2025

Conversation

trask
Copy link
Member

@trask trask commented Jan 25, 2025

Attempting to automate parts of open-telemetry/semantic-conventions#1817

This daily action will check if there's an existing opentelemetrybot/semantic-conventions-v* branch, and update that branch with the latest from main and update the semconv submodule with the latest from that repo's main.

If there's no existing opentelemetrybot/semantic-conventions-v* branch, it will create one and open a draft PR against it.

cc @open-telemetry/specs-semconv-maintainers

@trask trask requested a review from a team as a code owner January 25, 2025 01:09
Copy link
Member Author

Choose a reason for hiding this comment

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

removed since going in a different direction now

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

I would prefer that the scripts work on a PR branch rather than a dedicated and "permanent" repo branch. We don't need to preserve a continuous history of semconv integration as a part of this repo. A PR branch seems more appropriate IMHO.

Thoughts?

@trask
Copy link
Member Author

trask commented Jan 28, 2025

would naming it integration/semantic-conventions-1.31.0 be better, to be clear that we merge and delete it with each release?

I could have the script automatically create the branch (and associated PR) if it doesn't find one (meaning we've rolled over to the next release)

@chalin
Copy link
Contributor

chalin commented Jan 28, 2025

Yes, I'd like that: a branch per release that can be closed once the release has landed (hence it becomes a PR branch). Thanks!

@trask trask force-pushed the new-build-semconv-daily-workflow branch 2 times, most recently from 515595b to 7f2a912 Compare January 29, 2025 03:43
@trask trask force-pushed the new-build-semconv-daily-workflow branch from 7f2a912 to 5311ace Compare January 29, 2025 03:44
@trask trask changed the title Auto-update integration/semantic-conventions branch daily Update (or create) opentelemetrybot/semantic-conventions-v* branch daily Jan 29, 2025
@trask
Copy link
Member Author

trask commented Jan 29, 2025

Yes, I'd like that: a branch per release that can be closed once the release has landed (hence it becomes a PR branch). Thanks!

done, ptal, thanks!

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. See inline comments.

.github/workflows/update-semconv-integration-branch.yml Outdated Show resolved Hide resolved
.github/workflows/update-semconv-integration-branch.yml Outdated Show resolved Hide resolved
.github/workflows/update-semconv-integration-branch.yml Outdated Show resolved Hide resolved
git config user.name opentelemetrybot
git config user.email 107717825+opentelemetrybot@users.noreply.github.com

- name: Rebase
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I'm not sure that I'd want to automatically rebase the otel.io repo (that is what this is doing, right?). My gut feeling is that we might want to do this by hand. Maybe introduce a script bool argument to control this? Or just omit it and we can do it fully manually.

Copy link
Member Author

@trask trask Jan 30, 2025

Choose a reason for hiding this comment

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

it's merging from main (no force pushing involved), I can remove it if you'd prefer to do it by hand for now

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer rebasing, to keep the PR branch clean, and make it easier to cherry pick if/once we want to create a final PR.

But let's leave this as is for now. We can revisit later if necessary.


- name: Update submodule
run: |
git submodule update --init content-modules/semantic-conventions
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, we should be running npm run get:submodule, but that would require that in this step we not only update the submodule commit, but that we change the value of semconv-pin to correspond to the value returned by git describe --tags. Can we do that further below in this step?

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated semconv-pin now, but I'm not sure what npm run get:submodule is for, or when to run it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, npm run get:submodule is mainly to help local devs sync their submodules to the pinned versions. Come to think of it, we won't need it here.

.github/workflows/update-semconv-integration-branch.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Looking forward to giving this a spin.

@chalin chalin added this pull request to the merge queue Jan 30, 2025
Merged via the queue into open-telemetry:main with commit c78f66c Jan 30, 2025
17 checks passed
Vinaum8 pushed a commit to Vinaum8/opentelemetry.io that referenced this pull request Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants