Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

mark edit/unedit functionality as deprecated #3718

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tomerd
Copy link
Contributor

@tomerd tomerd commented Sep 4, 2021

motivation: edit/unedit functionality was put in place prior to the ability to do local overrides, making it redundant. it also add signfincant complexity to the code which could be good to remove

changes:

  • mark the public edit/unedit API as deprecated
  • emit deprecation warning on the edit/unedit CLI commands

motivation: edit/unedit functionality was put in place prior to the ability to do local overrides, making it redundant. it also add signfincant complexity to the code which could be good to remove

changes:
* mark the public edit/unedit API as deprecated
* emit deprecation warnin on the edit/unedit CLI commands
@tomerd
Copy link
Contributor Author

tomerd commented Sep 4, 2021

@swift-ci please smoke test

@tomerd tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Sep 4, 2021
@Argument(help: "The name of the package to edit")
var packageName: String

func run(_ swiftTool: SwiftTool) throws {
swiftTool.diagnostics.emit(.warning("Editing dependencies can be done via local overrides. 'edit/unedit' are no longer needed and will be deprecated soon."))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe there is a better way to describe this? When I search for “swiftpm local overrides”, the first hit is instructions for Xcode, a few hits are instructions to use this edit command, and the remainder are irrelevant.

Even after searching, I honestly don’t know how to do this on Linux without using edit.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's something we're going to implement as part of explicit overrides which are needed for the package registry support.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably the warning will have to come once there's a better implementation existing in SwiftPM CLI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is possible even without the introduction of explicit overrides by pointing the Package.swift to a local path instead of the URL, but I am happy to hold on to this PR to figure out of we want to do the explicit overrides feature first.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see — adding a path dependency in your root package. In that case the warning could be added now (I was thinking that this was only an Xcode thing).

@tomerd tomerd self-assigned this Sep 9, 2021
@tomerd tomerd added pending evolution proposal and removed ready Author believes the PR is ready to be merged & any feedback has been addressed labels Jan 25, 2022
@neonichu neonichu added the next waiting for next merge window label Apr 11, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
next waiting for next merge window pending evolution proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants