Skip to content

Deprecate Request.failableSend()#447

Merged
jpsim merged 6 commits into
masterfrom
nn-deprecate-request-send
Nov 30, 2017
Merged

Deprecate Request.failableSend()#447
jpsim merged 6 commits into
masterfrom
nn-deprecate-request-send

Conversation

@norio-nomura

@norio-nomura norio-nomura commented Nov 22, 2017

Copy link
Copy Markdown
Collaborator

Use Request.failableSend() instead of Request.send()
Change Request.send() to throws

Some APIs changed to throws:

  • File.format(trimmingTrailingWhitespace:useTabs:indentWidth:) throws
  • Structure.init(file:) throws
  • SyntaxMap.init(file:) throws

@norio-nomura norio-nomura force-pushed the nn-deprecate-request-send branch 2 times, most recently from 2e111f3 to bd6b04b Compare November 22, 2017 11:57
Comment thread CHANGELOG.md Outdated
* SourceKitten now requires Xcode 9 and Swift 3.2+ to build.
[Norio Nomura](https://github.com/norio-nomura)

* Deprecated `Request.send()`.

@jpsim jpsim Nov 22, 2017

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

"Please use Request.failableSend() instead."


let cursorInfo = Request.cursorInfo(file: swiftUUID, offset: usrOffset, arguments: compilerArguments).send()
guard let cursorInfo = try? Request.cursorInfo(file: swiftUUID, offset: usrOffset, arguments: compilerArguments).failableSend() else {
return nil

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

indentation

@jpsim jpsim left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍 other than two small comments.

@norio-nomura

Copy link
Copy Markdown
Collaborator Author

Thanks for reviews! Updated. ✨

@norio-nomura

Copy link
Copy Markdown
Collaborator Author

@jpsim your CircleCI account seems to be stopped by following error:

Your 14 day macOS trial has ended. Please go to the macOS plan page and select a plan to continue building.

https://circleci.com/gh/jpsim/SourceKitten/619

@jpsim jpsim force-pushed the nn-deprecate-request-send branch from 670c917 to f698d03 Compare November 28, 2017 22:12
@jpsim

jpsim commented Nov 28, 2017

Copy link
Copy Markdown
Owner

CircleCI support granted the jpsim GitHub org with access to the Seed plan in early October, but I've exchanged over a dozen emails with them since that point because this has never been reflected in the CircleCI settings dashboard.

I just got an email from them a few minutes ago asking to check to see if it was working now, so I rebased your last commit and pushed again to see if it would run, and it did.

So we should be all good now.

Thanks for the PR!

Comment thread CHANGELOG.md Outdated
* SourceKitten now requires Xcode 9 and Swift 3.2+ to build.
[Norio Nomura](https://github.com/norio-nomura)

* Deprecated `Request.send()`. Please use `Request.failableSend()` instead.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I feel like we should instead rename failableSend() to just send(). Throwing APIs in the Swift stdlib aren't prefixed with failable. It made sense to call it that when we had both, but now that we're removing that, I don't think it's appropriate anymore.

@norio-nomura norio-nomura changed the title Deprecate Request.send() Deprecate Request.failableSend() Nov 29, 2017
@norio-nomura norio-nomura force-pushed the nn-deprecate-request-send branch from db6ee8a to 0bccc94 Compare November 29, 2017 11:03
@norio-nomura

Copy link
Copy Markdown
Collaborator Author

Updated!

@jpsim jpsim left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I just merged #449 causing a conflict here. This is good to merge once that's addressed. Thanks!

@norio-nomura norio-nomura force-pushed the nn-deprecate-request-send branch from 0bccc94 to 2b015e8 Compare November 30, 2017 01:13
@norio-nomura

Copy link
Copy Markdown
Collaborator Author

Rebased. ✨

@jpsim jpsim merged commit d841db1 into master Nov 30, 2017
@jpsim jpsim deleted the nn-deprecate-request-send branch November 30, 2017 01:22
@norio-nomura

Copy link
Copy Markdown
Collaborator Author

Thanks! 🙏

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