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

Task/gen javadoc #650

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Task/gen javadoc #650

wants to merge 1 commit into from

Conversation

yannicklamprecht
Copy link
Collaborator

Checklist

  • I made changes to commands

    • I fully localised the command
    • I fully checked the commands functionality
  • I added new localisation codes

    • I fully translated it for all languages
    • I sorted properties alphabetically
  • I made changes to the database

    • I added my changed to a patch file
    • I made sure my database was up to date
    • I checked that the migration works
  • I made changes to internal structure

Short Description
Generated Javadoc

Detailed Description

Closes #

@yannicklamprecht yannicklamprecht changed the base branch from main to dev December 14, 2024 15:55
@yannicklamprecht yannicklamprecht force-pushed the task/gen-javadoc branch 22 times, most recently from 090ca97 to 511bd27 Compare December 14, 2024 21:00
@yannicklamprecht yannicklamprecht marked this pull request as ready for review December 14, 2024 21:11
@yannicklamprecht yannicklamprecht enabled auto-merge (squash) December 14, 2024 21:12
public MessageContext getCombinedContext(Message message, @Nullable Settings settings) {
return getCombinedContext(message.getMember(), message, settings);
}

/**
* Gets the combined context for the specified member and message.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
* Gets the combined context for the specified member and message.
* Gets the combined voice and message context for the specified member and message.

Comment on lines +26 to +52
/**
* Returns the result type of this analyzer result.
*
* @return the result type, which is always {@link ResultType#NO_MATCH}
*/
@Override
public ResultType resultType() {
return ResultType.NO_MATCH;
}

/**
* Converts this analyzer result to a snapshot.
*
* @return this analyzer result as a snapshot
*/
@Override
public ResultSnapshot toSnapshot() {
return this;
}

/**
* Adds this analyzer result to the provided embed builder.
*
* @param guild the guild where the result was generated
* @param entry the result entry
* @param builder the localized embed builder to add the result to
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure that documenting overridden methods again provides much value.

Comment on lines +36 to +41
/**
* Handles the slash command interaction event.
*
* @param event the SlashCommandInteractionEvent
* @param context the EventContext
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Same here and for all other command handlers. No need to document overrides

Comment on lines +19 to +21
/**
* Handler for the "add reaction" slash command, which adds a reaction to a message.
*/
Copy link
Owner

Choose a reason for hiding this comment

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

This is wrong

Comment on lines +19 to +21
/**
* Handler for the main slash command related to reactions.
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
/**
* Handler for the main slash command related to reactions.
*/
/**
* Handler to set the main reaction for reputation.
*/

Comment on lines +47 to +52
/**
* Retrieves command statistics for a specific week.
*
* @param week the week number
* @return the command statistics for the specified week
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
/**
* Retrieves command statistics for a specific week.
*
* @param week the week number
* @return the command statistics for the specified week
*/
/**
* Retrieves command statistics in a weekly resolution.
*
* @param week the week offset from the current week.
* @return the command statistics after the specified offset in weekly resolution
*/

Those functions are all super bad labeled. Its always about resolution and offset. this probably applies to all doc strings of metrics.

Comment on lines +54 to +58
/**
* Handles the event when the bot joins a guild.
*
* @param event the GuildJoinEvent
*/
Copy link
Owner

Choose a reason for hiding this comment

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

No need to document methods overridden from the ListenerAdapter

Comment on lines +104 to +108
/**
* Handles the message deletion event.
*
* @param event the message deletion event
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Same here with overrides

Comment on lines +137 to +141
/**
* Handles the event when a reaction emoji is removed from a message.
*
* @param event the message reaction remove emoji event
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Might tend to repeat myself. Will not do it for future listeners

@@ -20,21 +20,69 @@ private Colors() {
*/
@SuppressWarnings("unused")
public static final class Pastel {
/**
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure whether comments here add actual value to readability of the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants