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

Fixes #3344, Wrong iterator algorithm in PlotRangeIterator #3345

Merged
merged 2 commits into from Nov 23, 2021

Conversation

buepas
Copy link
Contributor

@buepas buepas commented Nov 23, 2021

Overview

Yeah dunno, its a very small change and I don't know, the issue says everything...

Description

This PR Fixes #3344

Checklist

  • [] Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
  • [] Ensure that the pull request title represents the desired changelog entry.
  • I tested my changes and approved their functionality.
  • I ensured my changes do not break other parts of the code
  • [] I read and followed the contribution guidelines

@buepas buepas requested a review from a team as a code owner November 23, 2021 04:05
@buepas
Copy link
Contributor Author

buepas commented Nov 23, 2021

also sorry if I didn't meet the standards. If you really want me to fit them for this one-liner, I can surely do that!

@SirYwell
Copy link
Member

Looks good. Could you add a test case in com.plotsquared.core.plot.PlotRangeIteratorTest to avoid regressions here? That would be very nice.

@NotMyFault NotMyFault added the Bugfix This PR fixes a bug label Nov 23, 2021
Copy link
Member

@NotMyFault NotMyFault 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, thanks for the contribution!

@buepas
Copy link
Contributor Author

buepas commented Nov 23, 2021

Looks good. Could you add a test case in com.plotsquared.core.plot.PlotRangeIteratorTest to avoid regressions here? That would be very nice.

Sure! When I come home I will do that

@buepas
Copy link
Contributor Author

buepas commented Nov 23, 2021

About the unit-tests:

It looks like that junit was upgraded to junit 5, but the imports haven't been updated?

Should this be handled in a different PR?

Edit:
I have written the test, and I converted the imports on my side.

@NotMyFault
Copy link
Member

Some tests are using JUnit 4, newer ones are using JUnit 5. Targeting the newer one is preferable. But no, you can append it to this PR.

Copy link
Member

@SirYwell SirYwell left a comment

Choose a reason for hiding this comment

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

Thank you very much!

@dordsor21 dordsor21 merged commit 8f3fa41 into IntellectualSites:v6 Nov 23, 2021
pull bot pushed a commit to Craftstuebchen/PlotSquared that referenced this pull request Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix This PR fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PlotRangeGenerator iterating wrong
4 participants