Skip to content

Conversation

fboemer
Copy link
Contributor

@fboemer fboemer commented Mar 21, 2025

Currently, the docs-check: workflow always adds the swift-docc-plugin dependency, forcing adopters to remove the dependency from their package. This PR edits the docs check to skip adding the dependency when already present.

@fboemer fboemer requested a review from a team as a code owner March 21, 2025 18:05
for package_file in $package_files; do
log "Editing $package_file..."
cat <<EOF >> "$package_file"
# yq 3.1.0-3 doesn't have filter, otherwise we could replace the grep call with "filter(.identity == "swift-docc-plugin") | keys | .[]"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

jq: error: filter/1 is not defined at <top-level>, line 1:[222](https://github.com/fboemer/swift-homomorphic-encryption/actions/runs/13997912746/job/39197486713#step:5:223)
.dependencies[].sourceControl | filter(.identity == "swift-docc-plugin") | keys | .[] 
jq: 1 compile error
yq: Error running jq: BrokenPipeError: [Errno 32] Broken pipe.

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. Looks good to me, just one small comment.

log "Editing $package_file..."
cat <<EOF >> "$package_file"
# yq 3.1.0-3 doesn't have filter, otherwise we could replace the grep call with "filter(.identity == "swift-docc-plugin") | keys | .[]"
hasDoccPlugin=$(swift package dump-package | yq -r '.dependencies[].sourceControl' | grep -e "\"identity\": \"swift-docc-plugin\"" || true )
Copy link
Member

Choose a reason for hiding this comment

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

swift package dump-package is JSON, so should this be jq instead of yq?

Also: Trailing space after true)

Copy link
Contributor Author

@fboemer fboemer Mar 21, 2025

Choose a reason for hiding this comment

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

We already use yq later on in the script, so I figured no need to an extra dependency, since yq supports json (as well as yaml)

Also: Trailing space after true)

Fixed

Copy link
Member

Choose a reason for hiding this comment

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

Oh, good call. Good with me.

@fboemer fboemer force-pushed the fboemer/append-docc-plugin-only-when-needed branch from 8107807 to a74cf14 Compare March 21, 2025 21:30
@FranzBusch FranzBusch merged commit ef1fb2e into swiftlang:main Mar 22, 2025
24 checks passed
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.

3 participants