Skip to content

Create ui for player armor, stats & spells#550

Draft
SohKey wants to merge 7 commits into
masterfrom
thomas/menu
Draft

Create ui for player armor, stats & spells#550
SohKey wants to merge 7 commits into
masterfrom
thomas/menu

Conversation

@SohKey

@SohKey SohKey commented Nov 26, 2022

Copy link
Copy Markdown
Member

see #526

Sceat
Sceat requested changes Nov 26, 2022
View reviewed changes

@Sceat Sceat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good overall, a few changes noticed ✨

I consider that you now have the level to provide clean git history, while we just squash other dev's PR, we will rebase this one! Please rewrite the full history of that PR with clean commits

Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js
Comment thread src/settings.js
Sceat
Sceat requested changes Nov 27, 2022
View reviewed changes
Comment thread src/armor_stand.js
Comment thread src/armor_stand.js
Comment thread src/player/menu.js Outdated
Comment thread src/player_menu.js Outdated
@SohKey SohKey self-assigned this Nov 27, 2022
@Sceat

Sceat commented Nov 28, 2022

Copy link
Copy Markdown
Member

Good overall, a few changes noticed ✨

I consider that you now have the level to provide clean git history, while we just squash other dev's PR, we will rebase this one! Please rewrite the full history of that PR with clean commits

Don't forget the git history

Sceat
Sceat requested changes Nov 28, 2022
View reviewed changes

@Sceat Sceat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I love it when you "finalize" the PR by creating twice more problems
image

Comment thread src/context.js
Comment thread src/freeze_player.js Outdated
Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js Outdated
Comment thread src/player/menu.js
Comment thread src/player_menu.js Outdated
Comment thread src/player_menu.js Outdated
Comment thread src/player_menu.js Outdated
Comment thread src/player_menu.js Outdated
Comment thread src/player_menu.js Outdated
@SohKey

SohKey commented Nov 28, 2022

Copy link
Copy Markdown
Member Author

Good overall, a few changes noticed ✨
I consider that you now have the level to provide clean git history, while we just squash other dev's PR, we will rebase this one! Please rewrite the full history of that PR with clean commits

Don't forget the git history

ooopsss

Sceat
Sceat requested changes Nov 29, 2022
View reviewed changes
Comment thread src/player/menu.js Outdated
Comment thread src/player_menu.js Outdated
Comment thread src/player_menu.js Outdated
Comment thread src/player_menu.js Outdated
@Sceat

Sceat commented Nov 29, 2022

Copy link
Copy Markdown
Member

Also npm run format and git history

@SohKey

SohKey commented Nov 29, 2022

Copy link
Copy Markdown
Member Author

Every lint commit is because i forgot to run format & lint 😭
About git history I promise to take care of it today 😎

@SohKey SohKey force-pushed the thomas/menu branch 6 times, most recently from 84f2ae2 to c99c468 Compare December 1, 2022 20:55
Sceat
Sceat requested changes Dec 18, 2022
View reviewed changes

@Sceat Sceat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • Branch out of date
  • Don't merge master, rebase on top of it
  • The first part of commits are not clean
  • Dependencies bump should not be included

Comment thread src/context.js Outdated
@SohKey SohKey linked an issue Dec 18, 2022 that may be closed by this pull request
Player menu (book) #579
Closed
@SohKey SohKey marked this pull request as draft December 21, 2022 10:18
@SohKey SohKey force-pushed the thomas/menu branch 3 times, most recently from 187e80e to 0a25f40 Compare December 22, 2022 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Sceat Sceat Sceat requested changes

@DeltaEvo DeltaEvo Awaiting requested review from DeltaEvo DeltaEvo is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

@SohKey SohKey

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Player menu (book)

3 participants

@SohKey @Sceat @DeltaEvo