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

Remove unused callback from GUI exercise #1293

Merged
merged 7 commits into from
Oct 3, 2023
Merged

Remove unused callback from GUI exercise #1293

merged 7 commits into from
Oct 3, 2023

Conversation

njr0
Copy link
Contributor

@njr0 njr0 commented Oct 3, 2023

Here's a possible update to the GUI/TUI exercise, taking out the callback and explaining (slightly).

See what you think.

njr0 and others added 4 commits October 2, 2023 14:34
Exercise 31.1 needs

use tempfile::tempfile;

to work, and also for `tempfile = "3"` to be added to Cargo.toml.
@google-cla
Copy link

google-cla bot commented Oct 3, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

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

This is nice, we should just take away the comments from the code.

src/exercises/day-3/simple-gui.rs Outdated Show resolved Hide resolved
@njr0
Copy link
Contributor Author

njr0 commented Oct 3, 2023

That's done. At least, I've updates the code in the branch and re-pushed. TBH, I don't use pull requests all that much, so I'm not certain whether I have to do anything else, but I can see the update on the main repo, so I presume you can too?

@mgeisler
Copy link
Collaborator

mgeisler commented Oct 3, 2023

That's done. At least, I've updates the code in the branch and re-pushed. TBH, I don't use pull requests all that much, so I'm not certain whether I have to do anything else, but I can see the update on the main repo, so I presume you can too?

The push is good, I also see the new changes here.

The only missing thing is the CLA bot (both here and in your other pull request). When the CLA is signed, then we can merge the PRs.

@mgeisler mgeisler changed the title Ex27 1 tui Remove unused callback from GUI exercise Oct 3, 2023
@njr0
Copy link
Contributor Author

njr0 commented Oct 3, 2023

I've signed a CLA.

Copy link
Collaborator

@mgeisler mgeisler left a comment

Choose a reason for hiding this comment

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

Looks good!

@mgeisler
Copy link
Collaborator

mgeisler commented Oct 3, 2023

I've signed a CLA.

Great, I told the bot to run the check again and it all looks good now. Thanks a lot for the help!

@mgeisler mgeisler enabled auto-merge (squash) October 3, 2023 14:03
@mgeisler mgeisler merged commit a883a56 into google:main Oct 3, 2023
30 checks passed
@njr0
Copy link
Contributor Author

njr0 commented Oct 3, 2023

Sorry, typo. I mean caller, not called!

@mgeisler
Copy link
Collaborator

mgeisler commented Oct 3, 2023

Sorry, typo. I mean caller, not called!

Ah, I did a best-effort on-the-fly update 😄 Feel free to submit a patch for this (I recommend using the little pencil icon in the top-right of the Drawing A Simple GUI page).

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