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

implemented rick and morty wiki #53

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Sasha-Bachynskyi
Copy link

No description provided.

Copy link

@yevheniisynytsia yevheniisynytsia left a comment

Choose a reason for hiding this comment

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

well done! small changes to go!

private final CharacterRepository characterRepository;
private final CharacterMapper characterMapper;
private final RickAndMortyClient rickAndMortyClient;
@Value("${rick-and-morty.get.characters.url}")

Choose a reason for hiding this comment

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

Suggested change
@Value("${rick-and-morty.get.characters.url}")
@Value("${rickAndMorty.characters.url}")


@PostConstruct
public void init() {
String url = getCharactersUrl;

Choose a reason for hiding this comment

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

url is redundant, just use the getCharactersUrl variable

Copy link
Author

Choose a reason for hiding this comment

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

done

spring.jpa.hibernate.ddl-auto=create-drop
spring.jpa.show-sql=true

rick-and-morty.get.characters.url=https://rickandmortyapi.com/api/character

Choose a reason for hiding this comment

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

Suggested change
rick-and-morty.get.characters.url=https://rickandmortyapi.com/api/character
rickAndMorty.characters.url=https://rickandmortyapi.com/api/character

at least get is odd. rickAndMorty or rick-and-morty or rick.and.morty is up to you

Copy link
Author

Choose a reason for hiding this comment

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

fixed

private final CharacterMapper characterMapper;
private final RickAndMortyClient rickAndMortyClient;
@Value("${rick-and-morty.characters.url}")
private String getCharactersUrl;

Choose a reason for hiding this comment

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

sorry didn't pay attention to it during the first review
getCharactersUrl means action, so it's more like a method name. Variables should be nouns, like charactersUrl

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