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

[FEATURE]: #2140: Add ArenaInfoPanel #2141

Merged
merged 1 commit into from
Oct 12, 2024
Merged

[FEATURE]: #2140: Add ArenaInfoPanel #2141

merged 1 commit into from
Oct 12, 2024

Conversation

sgcr
Copy link
Collaborator

@sgcr sgcr commented Aug 26, 2024

  1. changes proposed in this pull request:

  2. src/main/resources/release_notes.md ...

  • has been updated
  1. [Optional] suggested person to review this PR @___

@sgcr sgcr force-pushed the feature/arena-info-panel branch 2 times, most recently from a431b83 to 244f897 Compare August 26, 2024 21:31
wsbrenk
wsbrenk requested changes Aug 27, 2024
View reviewed changes
Copy link
Collaborator

@wsbrenk wsbrenk left a comment

Choose a reason for hiding this comment

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

Hi @sgcr, very good job.

src/main/java/hattrickdata/CurrentCapacity.java Outdated Show resolved Hide resolved
src/main/java/hattrickdata/ExpandedCapacity.java Outdated Show resolved Hide resolved
src/main/java/hattrickdata/HattrickDataInfo.java Outdated Show resolved Hide resolved
src/main/java/tool/arenasizer/ArenaInfoPanel.java Outdated Show resolved Hide resolved
wsbrenk
wsbrenk approved these changes Sep 3, 2024
View reviewed changes
Copy link
Collaborator

@wsbrenk wsbrenk left a comment

Choose a reason for hiding this comment

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

For me it is fine now - I don't want to decide about lombok or not...

@sgcr sgcr force-pushed the feature/arena-info-panel branch 4 times, most recently from 33d31a8 to f63d6a5 Compare September 3, 2024 19:53
@sgcr
Copy link
Collaborator Author

sgcr commented Sep 4, 2024

For me it is fine now - I don't want to decide about lombok or not...

Good :)

See also my comment regarding Java records with the last commit.

The pull requests: #2145 and #2143 should be reviewed and merged first because they are partly included here but separate general things.

I would like to rebase this PR here then.
#2145 is clearly a bug.
#2143 seems to be good idea due to testing.

@tychobrailleur
Copy link
Collaborator

I would like to rebase this PR here then. #2145 is clearly a bug. #2143 seems to be good idea due to testing.

Makes sense!

I had a quick glance at it this morning, at a high level, this looks like a good change, I just need to do the full code review this weekend.

@sgcr sgcr force-pushed the feature/arena-info-panel branch 2 times, most recently from 125aed8 to 8e04966 Compare September 10, 2024 21:25
@sgcr sgcr mentioned this pull request Sep 21, 2024
Merged
1 task
@sgcr sgcr force-pushed the feature/arena-info-panel branch 2 times, most recently from b91005e to aa4e757 Compare September 23, 2024 20:56
@sgcr
Copy link
Collaborator Author

sgcr commented Sep 29, 2024

Can be merged.

@wsbrenk wsbrenk merged commit 3cedbb2 into ho-dev:master Oct 12, 2024
1 check passed
@sgcr sgcr deleted the feature/arena-info-panel branch October 12, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wsbrenk wsbrenk wsbrenk approved these changes

@tychobrailleur tychobrailleur Awaiting requested review from tychobrailleur

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sgcr @tychobrailleur @wsbrenk