Skip to content

Resolve all current code warnings except the one about NIO #1148

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

Merged
merged 6 commits into from
Feb 11, 2025

Conversation

d-ronnqvist
Copy link
Contributor

Bug/issue #, if applicable:

Summary

This is the same changes as #1143 except that it doesn't include the Swift NIO update (so one warning remains in the project)

Dependencies

None.

Testing

Build DocC. There should only be 1 code warning.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • Added tests
  • Ran the ./bin/test script and it succeeded
  • Updated documentation if necessary

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@patshaughnessy patshaughnessy left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@@ -44,7 +44,6 @@ for language in swift-or-c bash md-or-tutorial html docker; do
declare -a matching_files
declare -a exceptions
declare -a reader
expections=( )
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this PR: it might be worth documenting the purpose and features of this script a bit. It seems useful and important, but some contributors might not be aware it exists or ever run it except as part of the bin/test.sh script.

@@ -136,6 +135,7 @@ EOF
cd "$here/.."
find . \
\( \! -path './.build/*' -a \
\! -path './bin/benchmark/.build/*' -a \
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this resolve a specific warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It resolves warnings about several files in Swift-Markdown that don't have the license header comment that DocC expects.

Currently, anyone who runs the benchmark tools (even just to read the help text) have to delete that build directory in order for bin/test to pass locally. This fixes that.

@@ -119,7 +119,12 @@ public class NavigatorTree {

func __read() {
let deadline = DispatchTime.now() + timeout
#if swift(>=5.10)
// Access to this local variable is synchronized using the DispatchQueue `queue`, passed as an argument.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation here.

_renderNodeProvider as! RenderNodeProvider?
}
// This property only exist to be able to assign `nil` to `renderNodeProvider` in the new initializer without causing a deprecation warning.
private let _renderNodeProvider: Any?
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the comment here; maybe also explain the use of Any is required to avoid the warning as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For code that's scheduled to be removed I feel that this is fine. The general oddity of the declaration is explained by the comment above and anyone who tries to change the type will notice that doing so reintroduces the deprecation warning.

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit af176cf into swiftlang:main Feb 11, 2025
2 checks passed
@d-ronnqvist d-ronnqvist deleted the fix-most-warnings-jan-2025 branch February 11, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants