Skip to content

Conversation

@trask
Copy link
Member

@trask trask commented Oct 9, 2025

@trask trask requested a review from a team as a code owner October 9, 2025 21:34
Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

Thanks @trask.

@laurit laurit added this pull request to the merge queue Oct 10, 2025
Merged via the queue into open-telemetry:main with commit bd55385 Oct 10, 2025
25 checks passed
@zeitlinger
Copy link
Member

I see that I inverted the check for "is PR"

This is what it was:

      - name: Link check (all files)
        if: github.event_name != 'pull_request' || steps.config-check.outputs.modified == 'true'

It would be easier to fix this script than all the repos that are using this script IMO

zeitlinger added a commit to zeitlinger/opentelemetry-java-contrib that referenced this pull request Oct 10, 2025
@zeitlinger zeitlinger mentioned this pull request Oct 10, 2025
@zeitlinger
Copy link
Member

Here's the alternative PR: #2345

@trask
Copy link
Member Author

trask commented Oct 10, 2025

Here's the alternative PR: #2345

This seems more complicated to me

It would be easier to fix this script than all the repos that are using this script IMO

I just tell copilot to port them, so not a problem to update all repos

@zeitlinger
Copy link
Member

I actually like that the current script is explicitly saying "not a PR, check everything"

That being said, I don't feel strongly about it.
Just don't remove the event type param, before the usages are gone - otherwise it'll break

@trask
Copy link
Member Author

trask commented Oct 10, 2025

Just don't remove the event type param, before the usages are gone - otherwise it'll break

oh, right, I forgot this repo's file is used elsewhere 👍

@trask trask deleted the daily branch October 20, 2025 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants