search

itsme-to / ItsMyConfig

A plugin that facilitates the work of configurators and adds functionalities to customize each message.
https://docs.itsme.to/itsmyconfig
Other
4 stars 5 forks source link

Refectoring & Documentation #4

Closed na2sime closed 4 months ago

na2sime commented 4 months ago

Pull Request

Summary

This pull request includes the following major changes:

(You can see more in each commits)

  1. Commit Hash: b1f6ece477ab9a2bb638ed568a9fbdc72e71fc8a

    • Author: NA2SIME
    • Description: This commit introduces extensive comments to the placeholder handling code, improving its readability and maintainability. The code has also been refactored to improve efficiency, particularly within PlaceholderManager, PlaceholderData, RequirementData, and other classes. Additionally, this change replaces hard-coded values with constants, providing better code management. Several methods and sections of code within the ItsMyConfig.java and DynamicPlaceHolder.java files were specifically updated.

  2. Commit Hash: b6c1f70506d53f540ac8a7a6dbc3897a8a749178

    • Author: NA2SIME
    • Description: This commit refactors classes for handling requirements and placeholders. The code has been elegantly adjusted and restructured, improving its manageability. This mainly includes unspecified changes to the Requirements and Placeholder related classes or methods.

Impact

This update mainly impacts the readability, maintainability, and efficiency of our placeholder and requirement handling code. By introducing comments, developers can now more easily understand our code, thereby reducing the time needed for onboarding and code reviews. Additionally, replacing hard-coded values with constants reduces the potential for error as there's no longer a need to remember and repeat specific values in multiple locations. Refactoring the code for requirement and placeholder handling presents a well-defined and more manageable structure to the code.

Testing

Testing was performed manually. The placeholder and requirement handling code was verified against a variety of cases to ensure the changes did not introduce new bugs and continued to meet our project's functional requirements. The refactored code has been confirmed to run more efficiently.

Approval

Requesting review and approval for these changes to be merged into the master branch. If approved, I'll merge this pull request.

iiAhmedYT commented 4 months ago

Code style is not fully-followed in most methods, a debug line is inside the code, but it's a really good pr

Note: I didn't mention everytime you didn't follow the code style

na2sime commented 4 months ago

I updated the pr, however could we add a linter to have a workflow & the same code style for everyone? Also add unit tests as well as a CI.

If you want I can take care of it

iiAhmedYT commented 4 months ago

I updated the pr, however could we add a linter to have a workflow & the same code style for everyone? Also add unit tests as well as a CI.

If you want I can take care of it

sure you can do that if you want, I approved this pull request and you can merge it if you’re ready. However I would recommend a different pr for these changes since they’re somewhat unrelated.. correct me if im wrong

na2sime commented 4 months ago

No, you are right, I have to create a new branch and a new PR for the linter and all the rest..

na2sime commented 4 months ago

Btw i can't merge you have to do it, i don't have permission

iiAhmedYT commented 4 months ago

Sure! Thanks for your work though <3