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.

Sign up for GitHub

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

Jump to bottom

Feat/customize makers #367

Merged
merged 9 commits into from
Mar 1, 2023
Merged

Feat/customize makers #367

merged 9 commits into from
Mar 1, 2023

Conversation

chihung93
Copy link
Contributor

I have added a customize makers for Current location and other location

@chihung93 chihung93 requested a review from ferranpons as a code owner October 25, 2022 08:34
@ferranpons
Copy link
Member

@chihung93 sorry for the late review.

First of all, I would like to thank you for your contribution. It's very welcome!

In order to properly review your PR please could add a screenshot with the final result?

Also, please revert the Readme changes because it's difficult to read it if you change all the file with no needed modifications.

Many thanks!

@chihung93
Copy link
Contributor Author

Hi @ferranpons , thanks for your response, My google account console is expired now, Im trying to find some key to take a screenshot it.

@chihung93
Copy link
Contributor Author

telegram-cloud-document-5-6325671410286986942.mp4

Hi @ferranpons , I record a video for this PR

ferranpons
ferranpons approved these changes Feb 6, 2023
View reviewed changes
Copy link
Member

@ferranpons ferranpons left a comment

Choose a reason for hiding this comment

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

Many thanks for your contribution. Please make the changes suggested and we'll merge the PR.

leku/src/main/java/com/adevinta/leku/LocationPickerActivity.kt Outdated Show resolved Hide resolved
@ferranpons
Copy link
Member

@chihung93 You need to correct these Ktlint issues to be able to merge:

Captura de pantalla 2023-02-08 a las 10 21 25

@chihung93
Copy link
Contributor Author

@chihung93 You need to correct these Ktlint issues to be able to merge:

Captura de pantalla 2023-02-08 a las 10 21 25

Screenshot 2023-02-08 at 23 59 32

Fixed! Thanks.

chihung93 and others added 2 commits February 9, 2023 00:03
@chihung93
Merge pull request #1 from AdevintaSpain/master
fc1f230 
Merging owner master to my master
Merge branch 'master' into feat/customize_makers
fcb1d04 
# Conflicts:
#	README.md
@ferranpons
Copy link
Member

ferranpons commented Feb 20, 2023 •
edited
Loading

@chihung93 The build is still not passing. Please review the lint issues:

Captura de pantalla 2023-02-20 a las 9 42 34

Checkout the build result here: https://github.com/AdevintaSpain/Leku/actions/runs/4126569199/jobs/7129972752

@chihung93
Copy link
Contributor Author

I have run ./gradlew :leku:ktlintMainSourceSetCheck . I believe that its good to go live.

Screenshot 2023-02-24 at 01 48 58

@ferranpons ferranpons merged commit 53a16b8 into AdevintaSpain:master Mar 1, 2023
@ferranpons
Copy link
Member

@chihung93 Many thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ferranpons ferranpons ferranpons approved these changes

@gerardpedreny gerardpedreny Awaiting requested review from gerardpedreny

@marcserrascmspain marcserrascmspain Awaiting requested review from marcserrascmspain

Assignees

@ferranpons ferranpons

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chihung93 @ferranpons