Skip to content
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

Update README.md #72

Closed
wants to merge 2 commits into from
Closed

Update README.md #72

wants to merge 2 commits into from

Conversation

mikekreuzer
Copy link

Update example Package.swift syntax

Update example Package.swift syntax
Copy link
Contributor

@QuietMisdreavus QuietMisdreavus 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 PR! I had a couple comments about your changes.

@@ -11,13 +11,13 @@ The markup tree provided by this package is comprised of immutable/persistent, t
In your `Package.swift` Swift Package Manager manifest, add the following dependency to your `dependencies` argument:

```swift
.package(url: "https://github.com/apple/swift-markdown.git", .branch("main")),
.package(url: "https://github.com/apple/swift-markdown.git", branch: "main"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a .package method that has a branch: parameter in the SwiftPM reference. Is this something that you needed to do to get this to build on your system?

Copy link
Author

Choose a reason for hiding this comment

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

It's there on the page you linked to :-)

Mostly I chose branch because that's what was there before, but there is a note on the pm reference that gives me pause:

/// Note that packages that use branch-based dependency requirements
/// can't be depended upon by packages that use version-based dependency
/// requirements; you should remove branch-based dependency requirements
/// before publishing a version of your package.

I don't know if the package manager really enforces that, it doesn't seem to, but we can't use version numbers here because the repo doesn't have semver numbered releases, just a lot of non semver tags.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't see it as an overload of the .package method. Instead i see:

static func package(url: String, from version: Version) -> Package.Dependency
static func package(url: String, _ requirement: Package.Dependency.Requirement) -> Package.Dependency
static func package(url: String, _ range: Range<Version>) -> Package.Dependency
static func package(url: String, _ range: ClosedRange<Version>) -> Package.Dependency
static func package(path: String) -> Package.Dependency

The way to define a branch-based dependency is to use the .branch method on Requirement and the .package(url:_:) overload that takes a Requirement, like the existing readme states. Does the form you've added, .package(url:branch:) work for you? I'm really curious to see if there's an undocumented overload there.

Copy link
Contributor

Choose a reason for hiding this comment

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

README.md Outdated Show resolved Hide resolved
Co-authored-by: QuietMisdreavus <QuietMisdreavus@users.noreply.github.com>
@Kyle-Ye
Copy link
Collaborator

Kyle-Ye commented Jul 14, 2023

Close in favor of #10

@Kyle-Ye Kyle-Ye closed this Jul 14, 2023
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.

None yet

4 participants