Skip to content

Conversation

@albyrock87
Copy link
Contributor

  • Add AutomationId to elements info: if the developer choose to set one it means it's an important info to discriminate a given element
  • Avoid reading the same bindable properties twice (minor perf improvement)

@codecov
Copy link

codecov bot commented Jun 4, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 72.87%. Comparing base (495e742) to head (c3a83e6).
Report is 636 commits behind head on main.

Files with missing lines Patch % Lines
src/Sentry.Maui/Internal/Extensions.cs 50.00% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4248      +/-   ##
==========================================
- Coverage   75.73%   72.87%   -2.86%     
==========================================
  Files         357      458     +101     
  Lines       13466    16644    +3178     
  Branches     2671     3318     +647     
==========================================
+ Hits        10198    12130    +1932     
- Misses       2593     3666    +1073     
- Partials      675      848     +173     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamescrosswell jamescrosswell marked this pull request as draft June 4, 2025 22:59
@jamescrosswell
Copy link
Collaborator

Thanks for the PR @albyrock87 !

Generally looks good. If you mark it ready for review, once it compiles and passes CI, we can take another look at this and get it merged in.

@albyrock87 albyrock87 marked this pull request as ready for review June 4, 2025 23:04
@albyrock87
Copy link
Contributor Author

The PR is ready for review, I have no idea why it started as a draft.

@jamescrosswell
Copy link
Collaborator

The PR is ready for review, I have no idea why it started as a draft.

I put it back into draft, since it doesn't yet pass any of the CI checks. It looks like a compilation error - I'm surprised it builds for you locally.

@albyrock87 albyrock87 force-pushed the add-automationid branch 2 times, most recently from c937fc3 to 46b5379 Compare June 5, 2025 09:20
@albyrock87
Copy link
Contributor Author

Something went definitely wrong with my commit :/
I've fixed the compilation error and rebased on top of main.

Copy link
Collaborator

@jamescrosswell jamescrosswell left a comment

Choose a reason for hiding this comment

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

Thanks again for the PR @albyrock87 !

@jamescrosswell jamescrosswell merged commit d47d66f into getsentry:main Jun 10, 2025
34 checks passed

- Rename MemoryInfo.AllocatedBytes to MemoryInfo.TotalAllocatedBytes ([#4243](https://github.com/getsentry/sentry-dotnet/pull/4243))
- Replace libcurl with .NET HttpClient for sentry-native ([#4222](https://github.com/getsentry/sentry-dotnet/pull/4222))
- Add .NET MAUI `AutomationId` element information to breadcrumbs ([#4248](https://github.com/getsentry/sentry-dotnet/pull/4248))
Copy link
Contributor

Choose a reason for hiding this comment

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

@jamescrosswell it seems like this got merged in the wrong place (it got added to the release notes for 5.10, instead of the Unreleased section). I noticed because I wanted to make sure my PR modified the changelog correctly.

It might be good to think about a way to better automate this process. Perhaps there is a way to have a bot automatically update the changelog, instead of having to do it manually. This way, the modifications always happen in the correct place, and not accidentally in already released sections.

Copy link
Contributor

Choose a reason for hiding this comment

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

Opened #4273 to fix this.

Copy link
Collaborator

@jamescrosswell jamescrosswell Jun 11, 2025

Choose a reason for hiding this comment

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

Thanks @KnapSac - much appreciated!

It might be good to think about a way to better automate this process. Perhaps there is a way to have a bot automatically update the changelog, instead of having to do it manually. This way, the modifications always happen in the correct place, and not accidentally in already released sections.

Yeah it's a bit tricky. The PR that updates the changelog in the main branch happens when making a release. We'd need something that went through and automatically updates any other open PRs... I imaging that could be done - just hard to find the time to look at it (given all the other issues on the backlog).

Copy link
Contributor Author

@albyrock87 albyrock87 Jun 11, 2025

Choose a reason for hiding this comment

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

I'm using release-drafter on my project and I'm very happy with it.
https://github.com/nalu-development/nalu/blob/main/.github/workflows/release-drafter.yml

I think it's much better than having a CHANGELOG.md file.
If we still want to have it, maybe some kind of automated process could pull the releases information from GitHub releases.

Copy link
Contributor

Choose a reason for hiding this comment

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

As a safe guard, maybe the GH action which checks for a changelog entry could also check that any modifications appear in the Unreleased section, and not in an already released section. It seems like that wouldn't be too hard to implement, but I'm not sure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm using release-drafter on my project and I'm very happy with it.

That might be an option if this was an isolated repo. The sentry-dotnet repo is one of over 650 in the getsentry org though so there's a standard release process used across all of the SDKs... it does stuff like updating the latest release version to be referenced in docs and download links etc.

As a safe guard, maybe the GH action which checks for a changelog entry could also check that any modifications appear in the Unreleased section, and not in an already released section. It seems like that wouldn't be too hard to implement, but I'm not sure.

There is a GH action to check that already but some new commits have to be pushed to a PR for it to trigger.

The problem occurs when this sequence of events occurs:

PR1: push changes (changelog entries correctly in unreleased)
-> All CI checks pass
PR2: push changes (changelog entries correctly in unreleased)
-> All CI checks pass
PR1: approve and merge
Create New Release (includes PR1)
-> Change log updated (Unreleased -> New Release)
-> CI checks don't run again on open PRs (i.e. PR2)
PR2: approve and merge
-> The change log entry is now unfortunately part of New Release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about having an UNRELEASED.md file?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure - I haven't spent time looking at it to see if that might work.

Releases happen here basically:

- name: Prepare release ${{ github.event.inputs.version }}
uses: getsentry/action-prepare-release@v1

That's a shared github action used by sentry-dotnet and the other repos, using Craft and tying into other things that are required to release stuff for Sentry.

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