-
-
Notifications
You must be signed in to change notification settings - Fork 2
Update documentation and templates. #105
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a comprehensive set of documentation and templates, which is a great step towards improving the project's accessibility for both users and contributors. The new issue templates, pull request template, contributing guidelines, and updated README are well-structured. I've provided a few suggestions to enhance clarity, fix broken links, and ensure consistency across these new files.
| Please follow these steps to have your contribution considered by the maintainers: | ||
|
|
||
| 1. Follow all instructions in [the template](.github/pull_request_template.md). | ||
| 2. Follow the [Checkstyle](checkstyle.xml) (if available) or standard Java coding conventions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link to the checkstyle.xml file appears to be broken. It should point to the correct location within the config directory to ensure contributors can find the style guide.
| 2. Follow the [Checkstyle](checkstyle.xml) (if available) or standard Java coding conventions. | |
| 2. Follow the [Checkstyle](config/checkstyle/checkstyle.xml) (if available) or standard Java coding conventions. |
| - label: I have read the CONTRIBUTING guidelines | ||
| required: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - label: I have read the CONTRIBUTING guidelines | ||
| required: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - [ ] Refactoring (no functional changes, just code cleanup) | ||
|
|
||
| ### Checklist | ||
| - [ ] My code follows the code style of this project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| ## Code of Conduct | ||
|
|
||
| This project and everyone participating in it is governed by our Code of Conduct. By participating, you are expected to uphold this code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Code of Conduct is mentioned but not linked. It's a best practice to include a link to a CODE_OF_CONDUCT.md file to ensure all participants can easily access it.
| This project and everyone participating in it is governed by our Code of Conduct. By participating, you are expected to uphold this code. | |
| This project and everyone participating in it is governed by our [Code of Conduct](CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code. |
| ### Player Commands | ||
| | Command | Usage | Permission | Description | | ||
| | :--- | :--- | :--- | :--- | | ||
| | `/balance` | `/balance [player]` | `eternaleconomy.player.balance` | Check your own account balance. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usage for checking one's own balance is documented as /balance [player], which can be confusing. Typically, checking one's own balance doesn't require any arguments. Changing the usage to just /balance would make it clearer.
| | `/balance` | `/balance [player]` | `eternaleconomy.player.balance` | Check your own account balance. | | |
| | `/balance` | `/balance` | `eternaleconomy.player.balance` | Check your own account balance. | |
| | `/economy set` | `/eco set <player> <amount>` | `eternaleconomy.admin.set` | Set a player's balance to a specific amount. | | ||
| | `/economy add` | `/eco add <player> <amount>` | `eternaleconomy.admin.add` | Add a specific amount to a player's balance. | | ||
| | `/economy remove` | `/eco remove <player> <amount>` | `eternaleconomy.admin.remove` | Remove a specific amount from a player's balance. | | ||
| | `/economy reset` | `/eco reset <player>` | `eternaleconomy.admin.reset` | Reset a player's balance to the default. | | ||
| | `/economy reload` | `/eco reload` | `eternaleconomy.admin.reload` | Reload the plugin configuration. | | ||
| | `/economy check` | `/eco check <player>` | `eternaleconomy.admin.balance` | Check a player's balance (Admin alias). | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The admin command documentation is inconsistent. The Command column lists /economy, but the Usage column uses the alias /eco. For clarity and consistency, it's better to use the primary command name /economy in the usage examples. The existence of aliases can be mentioned separately or left for users to discover in-game.
| | `/economy set` | `/eco set <player> <amount>` | `eternaleconomy.admin.set` | Set a player's balance to a specific amount. | | |
| | `/economy add` | `/eco add <player> <amount>` | `eternaleconomy.admin.add` | Add a specific amount to a player's balance. | | |
| | `/economy remove` | `/eco remove <player> <amount>` | `eternaleconomy.admin.remove` | Remove a specific amount from a player's balance. | | |
| | `/economy reset` | `/eco reset <player>` | `eternaleconomy.admin.reset` | Reset a player's balance to the default. | | |
| | `/economy reload` | `/eco reload` | `eternaleconomy.admin.reload` | Reload the plugin configuration. | | |
| | `/economy check` | `/eco check <player>` | `eternaleconomy.admin.balance` | Check a player's balance (Admin alias). | | |
| | `/economy set` | `/economy set <player> <amount>` | `eternaleconomy.admin.set` | Set a player's balance to a specific amount. | | |
| | `/economy add` | `/economy add <player> <amount>` | `eternaleconomy.admin.add` | Add a specific amount to a player's balance. | | |
| | `/economy remove` | `/economy remove <player> <amount>` | `eternaleconomy.admin.remove` | Remove a specific amount from a player's balance. | | |
| | `/economy reset` | `/economy reset <player>` | `eternaleconomy.admin.reset` | Reset a player's balance to the default. | | |
| | `/economy reload` | `/economy reload` | `eternaleconomy.admin.reload` | Reload the plugin configuration. | | |
| | `/economy check` | `/economy check <player>` | `eternaleconomy.admin.balance` | Check a player's balance (Admin alias). | |
|
|
||
| ## 🧩 Placeholders | ||
|
|
||
| These placeholders are used within the `messages.yml` file to dynamic content. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.