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.

Sign up for GitHub

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

Jump to bottom

internal: Deunwrap generate_derive #15434

Merged
merged 2 commits into from Aug 18, 2023
Merged

internal: Deunwrap generate_derive #15434

merged 2 commits into from Aug 18, 2023

Conversation

alibektas
Copy link
Member

#15398 subtask 1. Since the editing closure has arms, I did something experimental ( in this case just a clever term for bad code ) to bypass creating an Option but I am ready to change this.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 9, 2023
@alibektas alibektas changed the title Deunwrap generate_derive minor : Deunwrap generate_derive Aug 10, 2023
Veykril
Veykril reviewed Aug 15, 2023
View reviewed changes
Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

Looking for the derive upfront is fine, but let's defer the make calls to the closure again

crates/ide-assists/src/handlers/generate_derive.rs Show resolved Hide resolved
@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 15, 2023
@alibektas
Copy link
Member Author

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 18, 2023
@lnicola
Copy link
Member

lnicola commented Aug 18, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 18, 2023

📌 Commit 2a78173 has been approved by lnicola

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 18, 2023

⌛ Testing commit 2a78173 with merge c0f6ece...

@lnicola lnicola changed the title minor : Deunwrap generate_derive internal: Deunwrap generate_derive Aug 18, 2023
@bors
Copy link
Collaborator

bors commented Aug 18, 2023

☀️ Test successful - checks-actions
Approved by: lnicola
Pushing c0f6ece to master...

@bors bors merged commit c0f6ece into rust-lang:master Aug 18, 2023
10 checks passed
@lnicola lnicola mentioned this pull request Sep 9, 2023
Closed
15 tasks
@alibektas alibektas deleted the deunwrap/generate_derive branch September 29, 2023 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Veykril Veykril Veykril left review comments

@lnicola lnicola lnicola approved these changes

Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@alibektas @lnicola @bors @Veykril @rustbot