feat(macos): show notification updates #6679
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Bencher Report| Branch | feat/macos/update-notification |
| Testbed | github-actions |
🚨 1 ALERT: Threshold Boundary Limit exceeded!
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Lower Boundary (Limit %) | Upper Boundary (Limit %) |
|---|---|---|---|---|---|
| relayed-udp-server2client | Throughput bits/s | 📈 plot 🚨 alert 🚷 threshold | 314,402,011.07 (-6.55%) | 315,430,130.63 (100.33%) |
Click to view all benchmark results
| Benchmark | Throughput | Benchmark Result bits/s (Result Δ%) | Lower Boundary bits/s (Limit %) |
|---|---|---|---|
| direct-tcp-client2server | 📈 view plot 🚷 view threshold | 271,209,618.86 (+8.24%) | 232,082,905.99 (85.57%) |
| direct-tcp-server2client | 📈 view plot 🚷 view threshold | 281,411,458.29 (+10.12%) | 235,678,725.91 (83.75%) |
| direct-udp-client2server | 📈 view plot 🚷 view threshold | 317,510,636.89 (+8.49%) | 270,542,086.62 (85.21%) |
| direct-udp-server2client | 📈 view plot 🚷 view threshold | 489,638,447.54 (+20.11%) | 379,940,945.38 (77.60%) |
| relayed-tcp-client2server | 📈 view plot 🚷 view threshold | 266,806,482.60 (+6.27%) | 236,935,518.74 (88.80%) |
| relayed-tcp-server2client | 📈 view plot 🚷 view threshold | 286,650,494.10 (+9.42%) | 247,374,155.52 (86.30%) |
| relayed-udp-client2server | 📈 view plot 🚷 view threshold | 250,451,114.19 (+6.59%) | 219,077,045.59 (87.47%) |
| relayed-udp-server2client | 📈 view plot 🚨 view alert 🚷 view threshold | 314,402,011.07 (-6.55%) | 315,430,130.63 (100.33%) |
conectado
force-pushed
the
feat/macos/update-notification
branch
from
d78018c to
5e62d6a
Compare
thomaseizinger
approved these changes
Sep 14, 2024
Member
thomaseizinger
left a comment
thomaseizinger
left a comment
There was a problem hiding this comment.
Nice work! I left some feedback, nothing blocking :)
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
thomaseizinger
approved these changes
Sep 15, 2024
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
jamilbk
reviewed
Sep 16, 2024
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Show resolved
Hide resolved
jamilbk
reviewed
Sep 16, 2024
| } | ||
|
|
||
| private func startCheckingForUpdates() { | ||
| timer = Timer.scheduledTimer(timeInterval: 6 * 60 * 60, target: self, selector: #selector(checkForUpdates), userInfo: nil, repeats: true) |
Member
There was a problem hiding this comment.
Might be a tad more clear to make this a class constant above
Contributor
Author
There was a problem hiding this comment.
In Rust we keep the constants as close as possible to where they are used to make it easier to find the context, I think it's good if we keep the same standard across all our platforms.
jamilbk
reviewed
Sep 16, 2024
Comment on lines
124
to
126
| guard let versionString = versionString.data(using: .utf8) else { | ||
| return | ||
| } |
Member
There was a problem hiding this comment.
Shouldn't this guard be combined with the one above?
Comment on lines
128
to
130
| guard let versionInfo = try? JSONDecoder().decode(VersionInfo.self, from: versionString) else { | ||
| return | ||
| } |
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Show resolved
Hide resolved
…gainst other valid semantic versions
thomaseizinger
approved these changes
Sep 16, 2024
jamilbk
reviewed
Sep 17, 2024
swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift
Outdated
Show resolved
Hide resolved
…cation.swift Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
jamilbk
approved these changes
Sep 17, 2024
jamilbk
reviewed
Sep 17, 2024
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds a notification for updates for macos clients when they are on an old version.
This is how it looks:
The orange dot is shown regardless of the notification being dismissed.
If the notification is dismissed by the "Dismiss this version" button, until there's no new version there won't be notifications.
Updates are check at the start of firezone and every 6 hours after. This is saved in
UserDefaults.Permissions for notifications needs to be allowed so that it's show, this should be done by the
requestAuthorizationAlso, when an update is available a new
Update available...option appears in the menuThis option, same as the notification takes you to the appstore.