Skip to content

Replace asURL() call with URL(string:)#24350

Merged
kean merged 3 commits intotrunkfrom
mokagio/remove-asURL
Mar 28, 2025
Merged

Replace asURL() call with URL(string:)#24350
kean merged 3 commits intotrunkfrom
mokagio/remove-asURL

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Mar 28, 2025

Follows up to #24333, part of #24165.

The method comes from an old Alamofire version. Since its implementation, URL(string:) stopped failing on "invalid" strings such as "invalid-url". Between the different behavior and the cost of an additional dependency, the value of using that method is marginal at best.

Notice that the app still has an asURL() implementation, but that's part of the ParsedUrl custom type.

mokagio added 3 commits March 28, 2025 13:48
The method comes from an old Alamofire version. Since its
implementation, `URL(string:)` stopped failing on "invalid" strings such
as "invalid-url". Between the different behavior and the cost of an
additional dependency, the value of using that method is marginal at
best.
…age`

The method comes from an old Alamofire version. Since its
implementation, `URL(string:)` stopped failing on "invalid" strings such
as "invalid-url". Between the different behavior and the cost of an
additional dependency, the value of using that method is marginal at
best.
`asURL()` comes from an old Alamofire version. Since its
implementation, `URL(string:)` stopped failing on "invalid" strings such
as "invalid-url". Between the different behavior and the cost of an
additional dependency, the value of using that method is marginal at
best.

But aside from that, Swift now offers direct `URL` decoding, so we don't
need to jump through any hoop.

And notice that `StockPhotosMedia` has tests that validate the JSON
decoding.
@mokagio mokagio requested review from crazytonyli and kean March 28, 2025 03:01
@mokagio mokagio self-assigned this Mar 28, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Mar 28, 2025

1 Warning
⚠️ This PR is assigned to the milestone 25.9. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@mokagio mokagio added this to the 25.9 milestone Mar 28, 2025
@mokagio mokagio enabled auto-merge March 28, 2025 03:02
@wpmobilebot
Copy link
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number26827
VersionPR #24350
Bundle IDorg.wordpress.alpha
Commitd923f1a
Installation URL7vtoi2o2nq3t0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number26827
VersionPR #24350
Bundle IDcom.jetpack.alpha
Commitd923f1a
Installation URL3lbapb4g7jfvo
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

} else if let apiBase = apiBase(blog: blog),
let loginURL = try? blog.loginUrl().asURL(),
let adminURL = try? blog.adminUrl(withPath: "").asURL(),
let loginURL = URL(string: blog.loginUrl()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice.

@mokagio mokagio added this pull request to the merge queue Mar 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 28, 2025
@kean kean added this pull request to the merge queue Mar 28, 2025
Merged via the queue into trunk with commit 9ca0591 Mar 28, 2025
27 checks passed
@kean kean deleted the mokagio/remove-asURL branch March 28, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants