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

Improve documentation and comments #18

Open
AlexInCube opened this issue Oct 23, 2024 · 5 comments
Open

Improve documentation and comments #18

AlexInCube opened this issue Oct 23, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@AlexInCube
Copy link

Feature

I often don't understand what different methods or variables in a library do. I have to read the Riffy source code or use the debugger in the IDE to learn something.

Ideal solution or implementation

I suggest adding comments in the code to methods and variables.

For example:
/**
Represent in milliseconds current timeline position.
*/
public position: number;

And by the way, everywhere where time is specified, it seems that milliseconds are used, but it is not written anywhere about it.
For example in player.seek() method, the docs just say args is a "number".

Alternative solutions or implementations

This is example from Discord.js library, every property have description.
So I don't even need to go to the documentation, the IDE can show me what the variable means.

/**

  • A builder that creates API-compatible JSON data for slash commands.
    /
    declare class SlashCommandBuilder {
    /
    *
    • The name of this command.
      /
      readonly name: string;
      /
      *
    • The name localizations of this command.
      /
      readonly name_localizations?: LocalizationMap;
      /
      *
    • The description of this command.
      /
      readonly description: string;
      /
      *
    • The description localizations of this command.
      /
      readonly description_localizations?: LocalizationMap;
      /
      *
    • The options of this command.
      */

Other context

No response

@AlexInCube AlexInCube added the enhancement New feature or request label Oct 23, 2024
@AlexInCube AlexInCube changed the title Improve documentation Improve documentation and comments Oct 23, 2024
@AlexInCube
Copy link
Author

And variables like “player.position” should only be readonly.

@UnschooledGamer
Copy link
Member

it seems that milliseconds are used, but it is not written anywhere about it.
For example in player.seek() method, the docs just say args is a "number".

It's assumed as Riffy has the same properties Lavalink Gives, And I expect User must have seen the Lavalink docs at least once. And usually position like this Have in ms as it's easier to convert later.

@UnschooledGamer
Copy link
Member

And variables like “player.position” should only be readonly.

I cannot agree with it, As player.position needs to be updated everytime it changes and read-only is like constant in JS (cannot be updated again atleast for Number)

@UnschooledGamer UnschooledGamer self-assigned this Oct 24, 2024
@AlexInCube
Copy link
Author

AlexInCube commented Oct 24, 2024

And variables like “player.position” should only be readonly.

I cannot agree with it, As player.position needs to be updated everytime it changes and read-only is like constant in JS (cannot be updated again atleast for Number)

okay, you can make player.position private, but user can player.get_position()

@UnschooledGamer
Copy link
Member

okay, you can make player.position private, but user can player.get_position()

If we do that, We'll also need a setter as well. So currently it serves both purposes.

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

No branches or pull requests

2 participants