Skip to content

Add back accidentally removed API #152

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 22, 2021
Merged

Add back accidentally removed API #152

merged 2 commits into from
Feb 22, 2021

Conversation

glbrntt
Copy link
Contributor

@glbrntt glbrntt commented Aug 13, 2020 •
edited
Loading

Motivation:

Modifications:

  • Add back source-less API calls which defer to the decls with source.
  • Note we can't deprecate the calls we're adding back:
    log.info("some message") refers to the implementation we're adding
    back rather and would therefore require users to explicitly specify a source
    to suppress the deprecation warning message.

Result:

@glbrntt
Add back accidentally removed API
1815fd0 
Motivation:

- When `source:` was added in apple#135, the API was accidentally broken
  despite source compatability.

Modifications:

- Add back source-less API calls which defer to the decls with source.
- Note we can't deprecate the calls we're adding back:
  `log.info("some message")` refers to the implementation we're adding
  back rather and would therefore require users to explicitly specify a source
  to suppress the deprecation warning message.

Result:

- API is no longer broken.
@glbrntt
Copy link
Contributor Author

glbrntt commented Aug 13, 2020 •
edited
Loading

API breakages between 1.2.0 and 1.3.0:

❯ xcrun --toolchain org.swift.53202008101a ./scripts/check_no_api_breakages.sh . 1.3.0 1.2.0
Cloning into '/tmp/.check-api_zfKAaT/repo'...
done.
Checking public API breakages from 1.2.0 to 1.3.0
[4/4] Merging module Logging
[4/4] Merging module Logging
Checking Logging.json... ERROR
==============================
ERROR: public API change in Logging.json
==============================

/* Generic Signature Changes */

/* RawRepresentable Changes */

/* Removed Decls */
Func Logger.log(level:_:metadata:file:function:line:) has been removed
Func MultiplexLogHandler.log(level:message:metadata:file:function:line:) has been removed
Func StreamLogHandler.log(level:message:metadata:file:function:line:) has been removed

/* Moved Decls */

/* Renamed Decls */
Func Logger.critical(_:metadata:file:function:line:) has been renamed to Func critical(_:metadata:source:file:function:line:)
Func Logger.debug(_:metadata:file:function:line:) has been renamed to Func debug(_:metadata:source:file:function:line:)
Func Logger.error(_:metadata:file:function:line:) has been renamed to Func error(_:metadata:source:file:function:line:)
Func Logger.info(_:metadata:file:function:line:) has been renamed to Func info(_:metadata:source:file:function:line:)
Func Logger.notice(_:metadata:file:function:line:) has been renamed to Func notice(_:metadata:source:file:function:line:)
Func Logger.trace(_:metadata:file:function:line:) has been renamed to Func trace(_:metadata:source:file:function:line:)
Func Logger.warning(_:metadata:file:function:line:) has been renamed to Func warning(_:metadata:source:file:function:line:)

/* Type Changes */
Func Logger.critical(_:metadata:file:function:line:) has parameter 2 type change from Swift.String to () -> Swift.String?
Func Logger.critical(_:metadata:file:function:line:) has parameter 4 type change from Swift.UInt to Swift.String
Func Logger.debug(_:metadata:file:function:line:) has parameter 2 type change from Swift.String to () -> Swift.String?
Func Logger.debug(_:metadata:file:function:line:) has parameter 4 type change from Swift.UInt to Swift.String
Func Logger.error(_:metadata:file:function:line:) has parameter 2 type change from Swift.String to () -> Swift.String?
Func Logger.error(_:metadata:file:function:line:) has parameter 4 type change from Swift.UInt to Swift.String
Func Logger.info(_:metadata:file:function:line:) has parameter 2 type change from Swift.String to () -> Swift.String?
Func Logger.info(_:metadata:file:function:line:) has parameter 4 type change from Swift.UInt to Swift.String
Func Logger.notice(_:metadata:file:function:line:) has parameter 2 type change from Swift.String to () -> Swift.String?
Func Logger.notice(_:metadata:file:function:line:) has parameter 4 type change from Swift.UInt to Swift.String
Func Logger.trace(_:metadata:file:function:line:) has parameter 2 type change from Swift.String to () -> Swift.String?
Func Logger.trace(_:metadata:file:function:line:) has parameter 4 type change from Swift.UInt to Swift.String
Func Logger.warning(_:metadata:file:function:line:) has parameter 2 type change from Swift.String to () -> Swift.String?
Func Logger.warning(_:metadata:file:function:line:) has parameter 4 type change from Swift.UInt to Swift.String

/* Decl Attribute changes */

/* Fixed-layout Type Changes */

/* Protocol Conformance Change */

/* Protocol Requirement Change */
Func LogHandler.log(level:message:metadata:source:file:function:line:) has been added as a protocol requirement

/* Class Inheritance Change */
done

@glbrntt
Copy link
Contributor Author

glbrntt commented Aug 13, 2020

API breakages between 1.2.0 and this PR:

❯ xcrun --toolchain org.swift.53202008101a ./scripts/check_no_api_breakages.sh . HEAD 1.2.0
Cloning into '/tmp/.check-api_4E9ITe/repo'...
done.
Checking public API breakages from 1.2.0 to HEAD
[4/4] Merging module Logging
[4/4] Merging module Logging
Checking Logging.json... ERROR
==============================
ERROR: public API change in Logging.json
==============================

/* Generic Signature Changes */

/* RawRepresentable Changes */

/* Removed Decls */
Func MultiplexLogHandler.log(level:message:metadata:file:function:line:) has been removed
Func StreamLogHandler.log(level:message:metadata:file:function:line:) has been removed

/* Moved Decls */

/* Renamed Decls */

/* Type Changes */

/* Decl Attribute changes */

/* Fixed-layout Type Changes */

/* Protocol Conformance Change */

/* Protocol Requirement Change */
Func LogHandler.log(level:message:metadata:source:file:function:line:) has been added as a protocol requirement

/* Class Inheritance Change */
done

@glbrntt
Copy link
Contributor Author

glbrntt commented Aug 13, 2020

I think these two are okay because of the default implementation on the LogHandler

Func MultiplexLogHandler.log(level:message:metadata:file:function:line:) has been removed
Func StreamLogHandler.log(level:message:metadata:file:function:line:) has been removed

And this one is also fine for the same reason: it has a default implementation:

Func LogHandler.log(level:message:metadata:source:file:function:line:) has been added as a protocol requirement

glbrntt
glbrntt commented Aug 13, 2020
View reviewed changes
Tests/LoggingTests/LoggingTest.swift

logging.history.assertExist(level: .error, message: "errorDescription")
}
}

extension Logger {
public func error(_ error: Error,
public func error(error: Error,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fyi: I had to rename this so let error = logger.error(_:metadata:file:function:line:) wasn't ambiguous

@ktoso ktoso self-requested a review August 13, 2020 15:06
@ktoso
Copy link
Member

ktoso commented Aug 19, 2020 •
edited
Loading

(Sorry for being slow to get to this, will do soon -- want to make sure it's really correct this time)

@glbrntt
Copy link
Contributor Author

glbrntt commented Aug 19, 2020

(Sorry for being slow to get to this, will do soon -- want to make sure it's really correct this time)

No worries; I'm in no rush for this!

@ktoso ktoso changed the base branch from master to main September 3, 2020 12:35
ktoso
ktoso approved these changes Feb 22, 2021
View reviewed changes
Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Oh my, I was pretty bad to follow up here but LGTM.

Going to release with this today

@ktoso ktoso merged commit 12d3a86 into apple:main Feb 22, 2021
@glbrntt glbrntt deleted the gb-unbreak-api branch February 22, 2021 10:31
@saulbaro saulbaro mentioned this pull request Mar 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ktoso ktoso ktoso approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

API break in #135 (1.3.0)
2 participants
@glbrntt @ktoso