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

Feat: Player Rating Endpoints #8

Merged
merged 42 commits into from
Jun 19, 2024
Merged

Feat: Player Rating Endpoints #8

merged 42 commits into from
Jun 19, 2024

Conversation

invaderb
Copy link
Member

@invaderb invaderb commented May 6, 2023

feat: add ratings endpoints
feat: add scheduled job
test: add tests for rating generation
chore: add endpoints to readme

Adds the following endpoints to the service:
/v1/ratings
/v1/ratings/{player_name}

Changes:
Adds the following dependency @nest/schedule

feat: add scheduled job
test: add tests for rating generation
chore: ad endpoints to readme
@invaderb invaderb requested review from gruppler and humanat May 6, 2023 01:30
@invaderb invaderb self-assigned this May 6, 2023
Copy link
Collaborator

@nitzel nitzel left a comment

Choose a reason for hiding this comment

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

LGTM - just a few minor suggestions.

Should this be pointing to the dev branch?

src/app.module.ts Show resolved Hide resolved
src/models/ratings/ratings.module.ts Show resolved Hide resolved
src/models/ratings/ratings.service.spec.ts Outdated Show resolved Hide resolved
src/models/ratings/ratings.service.ts Outdated Show resolved Hide resolved
.env Show resolved Hide resolved
@invaderb invaderb changed the base branch from main to dev May 6, 2023 20:19
chore: add better api property descriptions
@invaderb invaderb requested review from nitzel and humanat November 11, 2023 16:45
README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@humanat humanat left a comment

Choose a reason for hiding this comment

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

Approving, assuming that the comments are trivial to explain.

src/main.ts Show resolved Hide resolved
invaderb added 2 commits June 18, 2024 18:07
chore: refactor to make variables more meaningful
chore: refactor ratings to have better typings
chore: refactor dto files
chore: update formatting
chore: fix typos
chore: fix typings
chore: clean up player dto
Copy link

@devp devp left a comment

Choose a reason for hiding this comment

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

LGTM but leaving some non-blocking comments for my understanding and some things I noticed. I most notably just skimmed over generateRating() because that was ported code that is hard to parse, and even riskier to rewrite without testing.

src/models/ratings/ratings.controller.ts Show resolved Hide resolved
src/models/ratings/ratings.service.ts Show resolved Hide resolved
src/models/ratings/ratings.service.ts Show resolved Hide resolved
src/models/ratings/ratings.service.ts Show resolved Hide resolved
src/models/ratings/ratings.service.spec.ts Show resolved Hide resolved
invaderb added 2 commits June 18, 2024 23:45
…en't changed

chore: add additional comments to explain the generateRatings
previous.txt Show resolved Hide resolved
@invaderb invaderb merged commit 66f9493 into dev Jun 19, 2024
1 check passed
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.

4 participants