Skip to content

Fix/remove argument-parser #6

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

Conversation

griffin-stewie
Copy link
Contributor

Summary

Remove the executable "markdown-tool" from the product so that ArgumentParser is not added unnecessarily when used it as a library.

In my case, I added swift-markdown to my CLI tool that it uses swift-argument-parser v1.0.1 then I got the following error.

Dependencies could not be resolved because root depends on 'swift-argument-parser' 0.4.4..<0.5.0 and root depends on 'swift-argument-parser' 1.0.0..<1.1.0.

This PR will fix this issue.

Dependencies

none.

Testing

All tests passed my local environment.

Checklist

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

  • Added tests
    • I don’t think we need to add tests for this PR.
  • Ran the ./bin/test script and it succeeded
    • added Package*.swift as exceptions
  • Updated documentation if necessary
    • I don’t think we need to add tests for this PR.

@QuietMisdreavus
Copy link
Contributor

Thanks for this! It looks like this shared some overlap with #13 - would you mind reworking this on top of the current main branch?

@griffin-stewie griffin-stewie force-pushed the fix/remove_ArgumentParser branch from 7923aa9 to cc99a57 Compare November 11, 2021 00:56
@griffin-stewie
Copy link
Contributor Author

@QuietMisdreavus
Thank you for reviewing my pull request.

I went over the current status of the main branch and what we wanted to do. Many of the changes I added were already included in the main branch, so they turned out to be simple changes.

Please review this pull request.

@ethan-kusters
Copy link
Contributor

@swift-ci please test

@@ -22,9 +22,6 @@ let package = Package(
.library(
name: "Markdown",
targets: ["Markdown"]),
.executable(
Copy link
Contributor

Choose a reason for hiding this comment

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

@griffin-stewie Thanks for doing this! Do you mind updating the base Package.swift while we're at it here? I think we should try to keep them consistent as much as possible.

…ntParser is not added unnecessarily when used it as a library.
@griffin-stewie griffin-stewie force-pushed the fix/remove_ArgumentParser branch from 6a36906 to 986cb14 Compare November 16, 2021 13:53
@griffin-stewie
Copy link
Contributor Author

@ethan-kusters
Thank you for reviewing, I have made the same fix to Package.swift as [email protected].

@ethan-kusters
Copy link
Contributor

@swift-ci please test

Copy link
Contributor

@ethan-kusters ethan-kusters left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you so much @griffin-stewie!

@ethan-kusters ethan-kusters merged commit c9dbdb5 into swiftlang:main Nov 16, 2021
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