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

Refactor: method name #54

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Refactor: method name #54

wants to merge 4 commits into from

Conversation

be-student
Copy link
Collaborator

@be-student be-student commented Jan 8, 2023

중요하진 않은 변경사항이기에
심사 후에 머지해도 상관 없습니다

@be-student be-student self-assigned this Jan 8, 2023
@Yaminyam Yaminyam changed the title Chore/method name Refactor: method name Jan 8, 2023
Comment on lines 11 to 16
async findByContent(search: string) {
return await this.createQueryBuilder('article')
.where('article.title LIKE :search', { search })
.where('article.content LIKE :search', { search })
.getMany();
}
Copy link
Member

Choose a reason for hiding this comment

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

byContent라고 하기에는 title이랑 content를 둘다 포함하고있어서 좀 애매하네요

Copy link
Member

Choose a reason for hiding this comment

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

content 말고 keyword 그런거로도 바꿀 수 있을 것 같아요

Copy link
Member

Choose a reason for hiding this comment

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

그냥 search로 둬도 될듯?

Copy link
Member

Choose a reason for hiding this comment

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

findByColumn 과 같은 이름으로 많이쓰니까 그거랑 아예 구분하기위해서

@@ -7,4 +7,11 @@ export class ArticleRepository extends Repository<Article> {
constructor(private readonly dataSource: DataSource) {
super(Article, dataSource.createEntityManager(), dataSource.createQueryRunner());
}

async search(search: string) {
Copy link
Member

Choose a reason for hiding this comment

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

다른 데는 타입 지정해놓으셔서 여기도 추가하면 좋을 듯 해서 코멘트 남깁니다

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.

3 participants