Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Team logos #529
Team logos #529
Changes from 8 commits
ea5e64d
39c3d98
63bcecf
cbfcc51
dfb1402
4f40d36
d2963b8
0623a62
a97e729
216c3ed
f245e33
bda3a07
3433c1e
59a6440
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 21 in backend/alembic/versions/19ddf67a4eeb_adding_teams_logo_paths.py
Codecov / codecov/patch
backend/alembic/versions/19ddf67a4eeb_adding_teams_logo_paths.py#L21
Check warning on line 25 in backend/alembic/versions/19ddf67a4eeb_adding_teams_logo_paths.py
Codecov / codecov/patch
backend/alembic/versions/19ddf67a4eeb_adding_teams_logo_paths.py#L25
Check warning on line 18 in backend/bracket/logic/teams.py
Codecov / codecov/patch
backend/bracket/logic/teams.py#L16-L18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to store it in a subdirectory
team_logos
actually, I should have done that too for the tournament logosThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want me to store the tournaments logos in a subdir too? I assume that's what you want so I'll commit that in if you don't reply to this before I make the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, I won't use
team_logos
but ratherteam-logos
as REST generally tends to prefer-
over_
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hesitant because it's a breaking change but let's just do it
Check warning on line 139 in backend/bracket/routes/teams.py
Codecov / codecov/patch
backend/bracket/routes/teams.py#L136-L139
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't it be
handleRequestError(response as AxiosError);
instead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, TypeScript complains about the insufficient overlap. I just added type defs to the axios library as it was annoying me that there weren't any. Also why is Axios required rather than imported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah no particular reason, must be an oversight.