-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add Extensive Logging #25005
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
Add Extensive Logging #25005
Conversation
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 30558 | |
| Version | PR #25005 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 4a4a631 | |
| Installation URL | 4fi7b4rofcg48 |
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 30558 | |
| Version | PR #25005 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 4a4a631 | |
| Installation URL | 1h4c1jlc6jotg |
|
Version |
540bef8 to
942aeec
Compare
|
Renamed it to "Developer Tools" |
WordPress/WordPress.xcodeproj/xcshareddata/xcschemes/Jetpack.xcscheme
Outdated
Show resolved
Hide resolved
| .font(.system(size: 20)) | ||
| .foregroundColor(.secondary) | ||
| .symbolEffect(.bounce.up, value: tapCount) | ||
| .onTapGesture { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does onTapGesture(count: 5) { isDeveloperModeEnabled = true } work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to change it and try it.
I generated all the original code.
|
If I understand correctly, with the changes in this PR, the app now always uses Pulse to log HTTP requests and responses, and the feature toggle controls whether the user can see those logs. Considering 99% of the users won't turn on the feature toggle and view the logs, to avoid running code and taking up the app's memory for nothing, should we change the model to only log HTTP requests when the feature toggle is on? |
There is a significant impact on performance and app on-disk see. If has to be a debug-only feature. |
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
|
I'm not sure you want to see the card on the most prime spot on the dashboard all the time you have logging enabled. It can be pretty annoying. Claude recommends the following as an ideal option:
|
|
@dcalhoun Good call. I'll align the content to the left to see how it looks.
@kean I agree with Jeremy's comment here ⬆️. I imagine users would only keep the extensive logging flag on for a short amount of time when they try to reproduce an issue. Maybe we can add an "Ignore" button to the card, which dismisses the card while keeping the extensive logging flag on? I think we should still warn users about it, though. We can make the "ignore" action take effect for a week or so, so users can still see the warning if they keep the flag on. |
|
Jeremy's concern was that it would turn itself off automatically, which is fair. With an alert, however, it won't. It will simply ask if you still need it. |
|
A persistent indicator also seems a OK, but I'd suggest showing it as a small badge with an info button instead of a large card. It only needs to say "Extensive Logging On". |
|
@kean Maybe something like this? Simulator.Screen.Recording.-.iPhone.17.-.2026-01-22.at.11.45.22.mov |
|
Here's another potential option, which I liked the most out of the ones I tried.
It should work well if the goal is to ensure you turn it off as soon as possible. I think we should let you hide it though like other cards. Look for how personalization service it used. It has the storage and the APIs for that. struct CompactLoggingBadge: View {
var body: some View {
HStack(spacing: 12) {
Circle()
.fill(.orange)
.frame(width: 8, height: 8)
VStack(alignment: .leading) {
Text("Extensive Logging Enabled")
.font(.subheadline)
.fontWeight(.medium)
Text("This feature may impact performance")
.font(.caption)
.foregroundStyle(.secondary)
}
Spacer()
Image(systemName: "info.circle.fill")
.foregroundStyle(.tertiary)
}
.padding(.vertical, 4)
}
}I'd show it under "Prompts" card so it doesn't draw as much attention, but is still visible. |
|
@kean Thanks for the suggestion! I have taken your code, with a small adjustment to the spacings to match the dashboard UI.
|
|












Description