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

Lidarr API client definition #718

Draft
wants to merge 4 commits into
base: new-ui-old
Choose a base branch
from

Conversation

anatosun
Copy link

@anatosun anatosun commented Jul 14, 2022

This is one of the first step towards Music integration into Petio.

EDIT:

The following PR is a work in progress. Its final goal is music integration into Petio. Here are tentative tasks (i.e., subject to being edited):

  • Lidarr API client definition
  • Lidarr Downloader definition
  • Musicbrainz integration
  • Frontend adjustments

Copy link
Collaborator

@ADRFranklin ADRFranklin left a comment

Choose a reason for hiding this comment

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

This is all and good, but I don't think this makes sense right now without the addition of the musicbrainz integration. It's just dead code with no actual use case.

}
}

export const GetRadarrInstanceFromDb = async (
Copy link
Collaborator

Choose a reason for hiding this comment

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

You forgot to rename the function from Radarr to Lidarr

// },
// ],
// response: ArtistsSchema,
// },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commented out code, shouldn't make it in to the codebase

@ADRFranklin
Copy link
Collaborator

I should also mention that we follow a strict commit message structure, and that is conventional commits. I'm surprised you were able to commit at all without husky or commit-cli complaining.

@anatosun
Copy link
Author

I made the necessary adjustments:

  • Removed the commented out code.
  • Renamed the function.
  • Changed the commit messages.
    Furthermore, I believe you're right about this PR not making total sense until the MusicBrainz integration. Let me convert this PR into draft.

@anatosun anatosun marked this pull request as draft July 16, 2022 14:00
@ADRFranklin
Copy link
Collaborator

ADRFranklin commented Jul 17, 2022

The commit messages still does not follow conventional commits. https://www.conventionalcommits.org/en/v1.0.0/

An example would be Added Lidarr API client definition becoming feat(lidarr): added api definitions

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