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

Finish client GUI #621

Merged
merged 5 commits into from
Jul 31, 2024
Merged

Finish client GUI #621

merged 5 commits into from
Jul 31, 2024

Conversation

770grappenmaker
Copy link
Contributor

Title. My first QM contribution, please be gentle!

Additional fixes:

  • Move SubtitlePositioner (not only GUI logic)
  • Remove invalid PacketReportProvider mapping (by gradle task)
  • Append website to wordlist

Additional fixes:
- Move SubtitlePositioner (not only GUI logic)
- Remove invalid PacketReportProvider mapping (by gradle task)
- Append `website` to wordlist
@IotaBread IotaBread added t: new adds new mappings v: release targets a release version of minecraft s: large PRs with more than 700 lines reviews needed please review this PR labels Jul 24, 2024
@OroArmor
Copy link
Member

Can you enable actions on your fork of the repository so that I can download the generated diff?

- Well, fastutil is still down, but at least it is *less* broken...
@770grappenmaker
Copy link
Contributor Author

Can you enable actions on your fork of the repository so that I can download the generated diff?

Should be enabled now. Since I pushed a new commit as well, it performs a fresh run of the workflows, which I couldn't get it to do with the old commit.

Copy link
Contributor

@supersaiyansubtlety supersaiyansubtlety 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, mostly just nitpicks

Copy link
Member

@ix0rai ix0rai left a comment

Choose a reason for hiding this comment

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

looks great, thank you so much for the huge PR! just a few suggestions

@770grappenmaker
Copy link
Contributor Author

Applied some of your suggestions, let me know what you think!

Copy link
Contributor Author

@770grappenmaker 770grappenmaker left a comment

Choose a reason for hiding this comment

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

Some thoughts of mine

Copy link
Member

@OroArmor OroArmor left a comment

Choose a reason for hiding this comment

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

Some small things

@770grappenmaker
Copy link
Contributor Author

Did not mean to rerequest @ix0rai for review (screw you github for moving the UI unexpectedly)

Copy link
Member

@ix0rai ix0rai left a comment

Choose a reason for hiding this comment

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

double approved

@770grappenmaker
Copy link
Contributor Author

When would this be eligible for merge?

@supersaiyansubtlety supersaiyansubtlety added final-comment-period is approved and will soon be merged if no issues are raised and removed reviews needed please review this PR labels Jul 29, 2024
@supersaiyansubtlety
Copy link
Contributor

whoops, I was supposed to start fcp when I approved, mb

@OroArmor OroArmor merged commit e110c7b into QuiltMC:1.21 Jul 31, 2024
3 checks passed
@770grappenmaker 770grappenmaker deleted the client-gui branch July 31, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period is approved and will soon be merged if no issues are raised s: large PRs with more than 700 lines t: new adds new mappings v: release targets a release version of minecraft
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants