-
Notifications
You must be signed in to change notification settings - Fork 16
SPM Prep – Rewrite RemoteBlogSettings
encoding and decoding in Swift
#780
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
Open
mokagio
wants to merge
10
commits into
trunk
Choose a base branch
from
mokagio/remote-blog-settings-swift
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
18069a2
Add test for `RemoteBlogSettings` init with JSON
mokagio 54ca822
Add test for `RemoteBlogSettings` encoding to dictionary
mokagio 23c2b34
Add note regarding the aim of `RemoteBlogSettingsTests`
mokagio 00f95c0
Implement init with JSON logic in `RemoteBlogSettings` in Swift
mokagio b0d9444
Add AI-generated test for all `RemoteBlogSettings` properties
mokagio de433d3
Implement `RemoteBlogSettings` conversion to `NSDictionary` in Swift
mokagio fb452a5
Remove redundant `RemoteBlogSettings` methods from `BlogServiceRemote…
mokagio bcb7f78
Use `[String: Any]` instead of `NSDictionary` in `RemoteBlogSettings`
mokagio 0ace60c
Restore a documentation comment that was lost in translation
mokagio 84df233
Remove `const`s that became unused from `BlogServiceRemoteREST`
mokagio File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Oops, something went wrong.
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.
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.
Maybe I worry too much, but do you think we should use
number(forKey:)
to parse all properties instead of just these keys, because that's what the original Objective-C uses?.We happen to know these weird cases, simply because we have one test json file for it. There is no indication in the original Objective-C code that these keys need special treatment. And, may be there are other keys needs a special "string" to "number" parsing, which is not detected by that one single test json file?
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.
Also, the
string(forKey:)
parsing in the original Objective-C code can parse a number to a string, which we don't do here. So, maybe we should usestring(forKey:)
andnumber(forKey:)
instead of type casting?