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

Update windowing chapter - Continue #24

Merged
merged 16 commits into from
Oct 2, 2023
Merged

Update windowing chapter - Continue #24

merged 16 commits into from
Oct 2, 2023

Conversation

Coddeus
Copy link
Contributor

@Coddeus Coddeus commented Oct 1, 2023 •
edited by marc0246
Loading

Started from Nekomansa/vulkano-book, applied suggested changes, and fixed a few more things for 0.33.0 (mainly imports and fmt).

Closes #17

@Coddeus Coddeus mentioned this pull request Oct 1, 2023
Closed
@Coddeus
Copy link
Contributor Author

Coddeus commented Oct 1, 2023

This should fix the 3 latest issues #10 #20 #22.

This was linked to issues Oct 1, 2023
Outdated example in Windowing - Swapchain Creation chapter #10
Closed
wrong language in creating a iterator of composite alphas #20
Closed
typo in Windowing 20.Swapchain creation #22
Closed
marc0246
marc0246 reviewed Oct 1, 2023
View reviewed changes
Copy link
Contributor

@marc0246 marc0246 left a comment

Choose a reason for hiding this comment

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

Looks very nice overall, just some nits regarding formatting. (The convention is that docs don't exceed 99 columns.)

@marc0246
Copy link
Contributor

marc0246 commented Oct 1, 2023

Also, I couldn't help but notice the change from *italic* to _italic_. We use the asterisks everywhere else so this is now inconsistent.

@Coddeus Coddeus force-pushed the update-windowing-chapter branch from 289cebe to 8dd0f4b Compare October 2, 2023 00:13
@Coddeus
Copy link
Contributor Author

Coddeus commented Oct 2, 2023

Fixed the underscores! I only realized I could batch the changes after I had done them -_-
Probably a squash merge commit would be best anyway.

@marc0246
Copy link
Contributor

marc0246 commented Oct 2, 2023

I only realized I could batch the changes after I had done them

No need to worry about that, we always squash-merge. :) Force-pushing usually does more harm than good.

marc0246
marc0246 reviewed Oct 2, 2023
View reviewed changes
Copy link
Contributor

@marc0246 marc0246 left a comment

Choose a reason for hiding this comment

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

Like here, I think you may have unintentionally undone some of the changes? You can apply them straight from the web interface to save you time.

@Coddeus @marc0246
Apply suggestions from code review
f96198d 
Co-authored-by: marc0246 <40955683+marc0246@users.noreply.github.com>
@Coddeus
Copy link
Contributor Author

Coddeus commented Oct 2, 2023 •
edited
Loading

Indeed, I couldn't find a way to push back what I changed locally.
Fixed it from the interface this time.

@marc0246
Copy link
Contributor

marc0246 commented Oct 2, 2023

Awesome! Thank you @Nekomansa and @Coddeus for the fixes ❤️, it's much appreciated!

@marc0246 marc0246 merged commit 707d3cb into vulkano-rs:main Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marc0246 marc0246 marc0246 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@Coddeus @marc0246 @Nekomansa