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

Code Refactoring #37

Closed
wants to merge 2 commits into from
Closed

Code Refactoring #37

wants to merge 2 commits into from

Conversation

MoMMde
Copy link

@MoMMde MoMMde commented Sep 25, 2021

Code refactoring

I just made the code smaller & moved to bukkit plugin acces

  • I have tested this pull request for defects on a server.

By making this pull request, I represent that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the TownyAdvanced organization has the copyright to use and modify my contribution under the SiegeWar License for perpetuity.

@MoMMde MoMMde changed the title moved to bukkit plugin access Code Refactoring Sep 25, 2021
@Goosius1
Copy link
Collaborator

  • Ok cool, looks nice
  • Did you test this on a local server?

@Goosius1
Copy link
Collaborator

  • I'm doing a release shortly
  • Since this code looks good to me, and since I will be testing the version before release now, I will merge

@Goosius1
Copy link
Collaborator

  • Oh one more thing, I've added the licence information to the PR description.
  • Can you confirm with a comment you are ok with the licence?, then I will merge

@Goosius1
Copy link
Collaborator

  • Tell you what, these improvements looks good to me, but lets keep them for 0.2.0 just to be safe.
  • Normally after we do pre-releases, we leave a few days to make sure changes are ok.
  • Given that today we're doing the first full release, I'm reluctant to put in anything except bugfixes.

@Goosius1 Goosius1 added the enhancement New feature or request label Sep 25, 2021
@Goosius1 Goosius1 added this to the 0.2.0 milestone Sep 25, 2021
@MoMMde
Copy link
Author

MoMMde commented Sep 25, 2021

  • I dont have any problem with the Licence 👍🏻
  • I sadly can't test it at the moment. Maybe i will send an update comment later

@Goosius1
Copy link
Collaborator

  • That's ok, we're not in a rush now.
  • So, the usual process is that the person who does the PR will test their changes on a local server (and tick the box).
  • Do you have a local server where you can do that?

@Goosius1 Goosius1 self-requested a review September 25, 2021 15:19
Copy link
Collaborator

@Goosius1 Goosius1 left a comment

Choose a reason for hiding this comment

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

  • Code looks good
  • No code changes requested, but a test of the code on a local server is requested.

@Goosius1
Copy link
Collaborator

Goosius1 commented Oct 1, 2021

Hi, can you test this?

@Goosius1
Copy link
Collaborator

Goosius1 commented Oct 3, 2021

  • PR closed as no response from PR opener.
  • If this work is still active then get in touch with us on the TownyAdvanced discord, and I'm sure we can re-open it and figure out what to do next
    • Depending on how long it is until you pick it up again, there may be merge conflicts.
    • After those are resolved it will need game-testing in the areas effected.
    • Usually this is done on a local server belonging to the PR-opener.
    • But if there's any issue with that, send us a test plan and we might have capacity to do it for you.

@Goosius1 Goosius1 closed this Oct 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants