-
Notifications
You must be signed in to change notification settings - Fork 9
feat: updated Point to be a sendable struct #71
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
base: master
Are you sure you want to change the base?
Conversation
abc99f2
to
dde52c8
Compare
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.
Hi @CraigSiemens, thanks for your PR.
I am fully OK with your changes. However, since this is a breaking change, please update the client version in Sources/InfluxDBSwift/InfluxDBClient.swift
to 2.0.0
. Additionally, please extend the description in the CHANGELOG.md
with information about how the API has changed and the benefits of these changes. Once that’s done, we can proceed to quickly merge your PR.
Best Regards
dde52c8
to
4137165
Compare
I've added the changed requested. I also updated the minimum swift version to 5.7 since that's the version that added the I also noticed the readme and examples are now going to be outdated. Is that something that should be updated in this PR too? |
Yes, we need to ensure the examples and README are aligned with the ‘current’ API, so please update those as well. |
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.
Please update README and examples.
4137165
to
bf91ec0
Compare
Deprecated addTag(key:value:), addField(key:value:), and time(time:) in favor of using the mutable properties. BREAKING CHANGE: Point is now a value type which can cause breaking behaviour in code expecting the behaviour of a reference type. BREAKING CHANGE: addTag(key:value:), addField(key:value:), and time(time:) have mutating versions when called and the result is discarded. BREAKING CHANGE: Minimum swift version is now 5.7
bf91ec0
to
5404a57
Compare
@bednar Could you re-review this PR and possibly give some insight on why the checks are failing?
Googling suggests the binary may be incompatible with the runner, though I ran the same steps on my M1 Mac and influxdb started with no issues. |
Thanks, your PR looks good, and I’m okay with it. Our team will look into fixing the CI, and after that, we’ll be able to merge this PR. |
Closes #69, closes #70
Proposed Changes
Updated
Point
to be a sendable struct.Deprecated addTag(key:value:), addField(key:value:), and time(time:) in favor of using the mutable properties.
Added mutating versions of addTag(key:value:), addField(key:value:), and time(time:) for use when the result is discarded. This is to try and minimize the number of complication errors introduced with this change. The following warnings will be produced in the following use cases.
Calling init and mutating functions in the same statement
Calling init and mutating functions in separate statements
Checklist
swift test
completes successfully