Skip to content
This repository has been archived by the owner on Feb 1, 2025. It is now read-only.

Update media to provide image #48

Merged
merged 13 commits into from
Mar 23, 2023
Merged

Update media to provide image #48

merged 13 commits into from
Mar 23, 2023

Conversation

MeKHell
Copy link
Contributor

@MeKHell MeKHell commented Mar 16, 2023

This PR will allow to directly have the image from the Media class, instead of the url provided until now.

@MeKHell MeKHell added the 4 Hour label Mar 16, 2023
@MeKHell MeKHell self-assigned this Mar 16, 2023
@MeKHell MeKHell requested a review from MariemBaccari March 17, 2023 07:09
Copy link
Contributor

@MariemBaccari MariemBaccari left a comment

Choose a reason for hiding this comment

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

Thank you for adressing my previous comment ! I suggested some other changes to make the code a bit cleaner but once that's done it would be ok for approval :)

@MeKHell MeKHell marked this pull request as draft March 22, 2023 20:20
@MeKHell MeKHell added 8 Hour and removed 4 Hour labels Mar 22, 2023
@codeclimate
Copy link

codeclimate bot commented Mar 22, 2023

Code Climate has analyzed commit 08c4801 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 73.5% (80% is the threshold).

This pull request will bring the total coverage in the repository to 60.4% (10.6% change).

View more on Code Climate.

@MeKHell MeKHell marked this pull request as ready for review March 22, 2023 21:46
@MeKHell MeKHell closed this Mar 22, 2023
@MeKHell MeKHell reopened this Mar 23, 2023
Copy link
Contributor

@MariemBaccari MariemBaccari left a comment

Choose a reason for hiding this comment

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

Good job with the viewModel and the adapters ! I'm glad the API is working now. I see you also adressed most of my change requests. As we discussed previously, given the fact that we need to urgently merge, this would be an intermediary PR so we would expect another one that tests the feature more thoroughly and adresses the comment about the tests.

@MariemBaccari MariemBaccari merged commit af6bbc9 into main Mar 23, 2023
@MariemBaccari MariemBaccari deleted the link_API_Media branch March 24, 2023 11:05
@MeKHell MeKHell restored the link_API_Media branch March 24, 2023 17:42
@MeKHell MeKHell mentioned this pull request Mar 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants