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

fix: merge two constructors in footerlink #36

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

DaleSeo
Copy link
Contributor

@DaleSeo DaleSeo commented Jun 11, 2024

웹사이트를 둘러보다가 우연히 콘솔에서 다음 오류를 발견하였습니다. 간단한 버그라서 바로 고칩니다.

Shot 2024-06-11 at 17 59 21@2x

@DaleSeo DaleSeo requested a review from a team as a code owner June 11, 2024 22:06
@sounmind
Copy link
Member

sounmind commented Jun 11, 2024

#31 여기서 병현님이 직접 고쳐보시는 게 좋을 것 같아서 이슈 만들었었는데, 그냥 바로 병합할까요? @DaleSeo


attributeChangedCallback(name, oldValue, newValue) {
if (name === "href") {
this.updateLinkHref(newValue);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

생성자를 병합했더니, 여기에 메서드 이름에도 오타 발견되어 같이 정정하였습니다. updateLinkHref() -> updateLink()

@DaleSeo DaleSeo linked an issue Jun 11, 2024 that may be closed by this pull request
@DaleSeo
Copy link
Contributor Author

DaleSeo commented Jun 11, 2024

#31 여기서 병현님이 직접 고쳐보시는 게 좋을 것 같아서 이슈 만들었었는데, 그냥 바로 병합할까요?

@sounmind 아이고 제가 이슈를 미리 확인했어야 했는데... 죄송합니다 😅
개인적으로 main 브랜치에 있는 이슈는 가급적 즉시 수정하는 게 좋다고 생각합니다. 아니면 다른 브랜치에도 악영향을 줄테니까요.
어차피 제가 의도치 않게 시간을 썼는데 해당 이슈를 이 PR에 연결하고 빨리 조치하시는 게 어떠신가요?

@sounmind
Copy link
Member

#31 여기서 병현님이 직접 고쳐보시는 게 좋을 것 같아서 이슈 만들었었는데, 그냥 바로 병합할까요?

@sounmind 아이고 제가 이슈를 미리 확인했어야 했는데... 죄송합니다 😅 개인적으로 main 브랜치에 있는 이슈는 가급적 즉시 수정하는 게 좋다고 생각합니다. 아니면 다른 브랜치에도 악영향을 줄테니까요. 어차피 제가 의도치 않게 시간을 썼는데 해당 이슈를 이 PR에 연결하고 빨리 조치하시는 게 어떠신가요?

넵 동의합니다. PR content에 close Issue를 해놓을게요!

@DaleSeo DaleSeo merged commit efd5900 into main Jun 11, 2024
1 check passed
@DaleSeo DaleSeo deleted the fix-two-constructors-in-footerlink branch June 11, 2024 22:29
@bhyun-kim
Copy link
Contributor

제가 조금 일찍 확인했어야하는데 늦었네요 ㅠㅠ. 두 분 모두 확인해주셔서 감사드립니다.

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.

Footer Link Component Bug fix website title not working bug
3 participants