Skip to content

Conversation

@fatkodima
Copy link
Contributor

Follow up to #1443.

It is perfectly fine to have unowned files, but appsignal currently produces log lines for this case, which look like some errors and can be confusing

INFO -- : ║ [2025-09-30T18:13:32 (process) #8][ERROR] appsignal: Error while looking up CodeOwnership team: NoMethodError: undefined method `name' for nil

@unflxw unflxw requested review from lipskis and unflxw October 1, 2025 08:16
@unflxw
Copy link
Contributor

unflxw commented Oct 1, 2025

Thanks for the PR @fatkodima! I believe you're correct that this should not emit an error log line.

@unflxw unflxw requested a review from tombruijn October 1, 2025 08:17
Copy link
Contributor

@unflxw unflxw left a comment

Choose a reason for hiding this comment

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

Looks good to me. Let's make sure to add a changeset for it after merging.

Copy link
Contributor

@lipskis lipskis left a comment

Choose a reason for hiding this comment

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

Looks good to me as well, thanks @fatkodima!

Copy link
Member

@tombruijn tombruijn left a comment

Choose a reason for hiding this comment

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

Thanks!

+1 on adding the changeset after merge.
Let's merge the PR with a squash commit that contains the PR description as the commit message.

@lipskis lipskis merged commit 71469de into appsignal:main Oct 1, 2025
189 of 198 checks passed
@fatkodima fatkodima deleted the handle-unowned-files branch October 1, 2025 09:17
lipskis added a commit that referenced this pull request Oct 1, 2025
Add a changeset for #1463, which is a community contribution.

[skip review]
lipskis added a commit that referenced this pull request Oct 1, 2025
lipskis added a commit that referenced this pull request Oct 1, 2025
Add a changeset for #1463, which is a community contribution.

[skip review]
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