Skip to content
This repository was archived by the owner on Sep 15, 2025. It is now read-only.

Modernize NSDate and Date implementation introducing breaking changes#759

Merged
mokagio merged 4 commits intomokagio/rfc-3339from
mokagio/rfc-3339-breaking-change
Mar 21, 2024
Merged

Modernize NSDate and Date implementation introducing breaking changes#759
mokagio merged 4 commits intomokagio/rfc-3339from
mokagio/rfc-3339-breaking-change

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Mar 20, 2024

Follows up on @crazytonyli comments from the PR #758, which it targets.

I don't know why I felt like making a dedicated PR instead of pushing to that one directly 🤷‍♂️ Maybe because of the breaking changes...


  • Please check here if your pull request includes additional test coverage. — N.A.
  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary. — N.A.

var wordPressComJSONString: String {
DateFormatter.wordPressCom.string(from: self)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like these extension functions don't really help much. They can just use the formatter instance DateFormatter.wordPressCom directly, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I just noticed wordPressCom is an internal API. Please ignore my comment above, if you intend to not expose the DateFormatter as public API, which I think is a good idea (leaving us option to switch over to ISO8601DateFormatter).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Yes. This whole custom formatting feels odd. I think once we get the code in a better place in term of SPM, we could look at trimming some of those unnecessary custom bits in favor of standards

Copy link
Contributor

@crazytonyli crazytonyli left a comment

Choose a reason for hiding this comment

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

I'm okay with merging these two PR together as one. It might be easier for code review 🙈

@mokagio mokagio merged commit 2a90451 into mokagio/rfc-3339 Mar 21, 2024
@mokagio mokagio deleted the mokagio/rfc-3339-breaking-change branch March 21, 2024 06:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants