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

리팩토링 #214

Open
disjukr opened this issue Nov 28, 2015 · 5 comments
Open

리팩토링 #214

disjukr opened this issue Nov 28, 2015 · 5 comments

Comments

@disjukr
Copy link
Owner

disjukr commented Nov 28, 2015

  1. 구현들이 지금 이름없는 파싱 함수 하나만 export default function으로 export하고 있는데, 해당 함수를 export function parse로 바꾸고, jews.cleanup을 별도로 export function cleanup으로 분리하려고 합니다.
  2. 속성들의 타입을 강제하기 위해 parse 함수에서 let jews = {}를 하는 대신, let jews = new Jews()로 바꿀 생각입니다. ( https://github.com/disjukr/jews/blob/master/src%2Fjews.user.js#L6 )
  3. jews boost #190 이 이슈와 관련지어서 작업하고 싶습니다. 해당 이슈에서는 구현마다 export function boost를 하나 만들고 jews가 돌아도 괜찮은 시점에 true를 반환하도록 작업하면 될 것 같습니다.
@disjukr
Copy link
Owner Author

disjukr commented Feb 10, 2018

3 - readyToParse

@disjukr
Copy link
Owner Author

disjukr commented Feb 14, 2019

@sunho 이 이슈는 만든지 오래되어서 좀 outdated된 부분이 있는 것 같아요. 디스코드에서 어떻게 재설계하는게 좋을지 같이 얘기해봐요.

@sunho
Copy link
Collaborator

sunho commented Feb 22, 2019

@disjukr 커미터 분들도 의견을 낼 수 있게 여기서 얘기하죠.

@sunho
Copy link
Collaborator

sunho commented Feb 22, 2019

일단 제가 작업하면서 떠오른 개선안을 나열해볼게요.

  1. 아티클, 리포터 타입을 index에서 분리

  2. 파서 구현체를 추상 클래스에서 상속받도록 (인터페이스)

parse 함수 이름 스팰링같은걸 틀렸을 때 타입 오류로 뜨면 좋을 것 같습니다.

  1. 테스트 만들기

구현체들의 동작 여부를 테스트를 돌려서 한눈에 파악할 수 있으면 좋을 것 같습니다. 많은 노가다가 필요하지만...

  1. 에러 처리 개선

이건 제가 자스 경험이 적어서 정확히 어떤 상황인 줄 모르겟네요.

  1. XMLHttpRequest대신 fetch쓰기

  2. Article 타입에서 undefined를 안 써도 되도록

subtitle 필드에다가 가끔 undefined를 넣는 파서 구현체들이 있었는데 좀 아리송한 것 같습니다.

  1. 충격과 공포의 스포츠동아와 미디어스 리팩토링

얘네 너무 무서워요...

@disjukr
Copy link
Owner Author

disjukr commented Feb 22, 2019

@sunho

커미터는 저장소에 커밋할 권한이 있는 사람을 뜻하는, 요즘에는 거의 쓰이지 않거나 잘못 쓰이고 있는 용어입니다.
(옛날에는 요즘의 깃헙처럼 수정사항을 직접 커밋하여 PR 보내는 방식이 아닌, 커미터에게 diff 텍스트를 메일로 보내서 커밋을 부탁하는 방식을 사용하였습니다.)

아마 컨트리뷰터를 말씀하고 싶으셨을 것 같습니다.


  1. 네 그러죠. src/type.ts 정도로 분리하면 될 것 같습니다.
  2. 전에는 Article 클래스를 두어 상속받는 모양을 생각했는데 지금은 안그랬으면 좋겠어요. 그냥 지금의 Article 인터페이스 타입으로 충분한 것 같습니다.
    이유는 plain js object라면 직렬화 역직렬화 등이 수월하여 이후 엔지니어링에 고민할 부분이 적어지는 점을 들 수 있습니다.
  3. 별도 이슈로 다뤄야할 것 같습니다.
  4. 문제가 어떤 것인지 명확히 할 필요가 있을 것 같습니다.
  5. 네 그러죠.
  6. js에서 key가 없는 것과 key는 있는데 undefined 값이 대입되어있는 것의 의미는 다릅니다. just-news에서는 그 둘을 엄밀하게 구분하지 않지만, 각각의 파서 구현체 파일들이 작성한 분들의 스타일에 따라 다를 뿐입니다.
    이는 코딩 컨벤션을 정함으로 해결할 수 있을 것 같습니다만 저는 코딩 컨벤션을 엄격하게 두게 되면 기여하는데 진입장벽을 만들 수 있지 않을까 하는 걱정이 있습니다.
  7. 저도 잘은 모르는데, 스포츠동아는 일단 이미지 슬라이드를 처리하는 코드가 있는 것 같고, 미디어스에서는 사이트가 자동으로 스크롤되도록 하지 못하게 방지하는 처리가 되어있는 듯 합니다.
    미디어스의 자동스크롤 방지 처리는 src/reconstruct.ts에서 처리하도록 변경해도 좋을 것 같네요.

그리고 사이트 로딩을 전부 기다리지 않아도 되도록 하는 최적화를 전체적으로 작업하고 싶습니다.
일단 기반은 만들어져 있으며 (사이트마다 readyToParse 함수를 구현하면 됩니다.) 자세한 내용은 #305 (comment) 이 곳의 대화내용을 참고하면 좋을 것 같습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants