search

vemurikarthik / vue-mini-player

A simple mini player powered by petite-vue
MIT License
0 stars 0 forks source link

Wrapper around song list #79

Closed 0xDTE closed 1 year ago

0xDTE commented 1 year ago

This a feature of a wrapper round the song list as per the new UI mocks on figma.

vemurikarthik commented 1 year ago

torvalds.dev is analyzing the pull request

vemurikarthik commented 1 year ago

The Pull Request diff indicates the following files have been modified and added:

Modified Files:

Added Files:

Based on the provided coding standards document and the Github PR diff, here are the violations and recommendations:

  1. Naming Conventions:

    • Various variables are renamed to single letter ones which is not a good practice as per the standards document. Examples can be found in the changes to file src/core/VueMiniPlayerCore/index.ts and src/core/MusicPlayerCore/index.ts where variables are renamed to 'a', 'b', 'P' etc. This is not recommended as such variable names are not descriptive and do not provide context or meaning.

    Recommendation: Change back these variable names to more descriptive ones.

  2. Code Formatting:

    • A function name is changed from 'PrevSong' to 'Prev-Song'. This does not follow the recommended camel case format for functions.

    Recommendation: Change it back to 'PrevSong'.

  3. Indentation and Whitespace

    • There seems to be unnecessary whitespace added in file src/core/MusicPlayerCore/index.ts under the 'AppendSongOnHead' function. Whitespace should be used sparingly.

    Recommendation: Remove extraneous whitespace.

  4. Syntax issue:

    • In the modification of file src/core/MusicPlayerCore/index.ts, the statement 'this.SongIdList === list' appears to be intending to make an assignment but is instead making a comparison due to using '==='

    Recommendation: If intending to make an assignment, change it to 'this.SongIdList = list'.

  5. Consistency:

    • The changes seem inconsistent - for example 'p' is changed to 'P' in one place but remains as 'p' in another, and 'WrapperID' is changed to 'a' in one file but remains as 'WrapperID' in the new index_2.ts file.

    Recommendation: Strive for naming consistency across the entire source code.

Without executing any code, these issues have been surfaced from manual review of the PR diff. Remember that this is a preliminary review and more issues may exist. Automated static code analysis and peer review can help identify more issues.