-
Notifications
You must be signed in to change notification settings - Fork 0
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
[feat] rss 변환 함수 구현 #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다. 테스트 코드도 추가해주시면 동작 결과를 바로 확인할 수 있어서 좋을 것 같아요!
src/core/rss.rs
Outdated
ItemBuilder::default() | ||
.title(value.title) | ||
.link(value.url) | ||
.pub_date(value.created_at.format(&Rfc2822).unwrap()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3) updated_at 필드도 추가되어야 할 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rss spec상 update date 표시가 불가하더라고요. 기준을 updated_at으로 변경해야 할까요?
참고
https://www.rssboard.org/rss-specification#hrelementsOfLtitemgt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
흐으음 왜 업데이트 태그가 없지..
그럼 일단 그대로 두는 게 나을 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extensions()
메서드를 활용하면 확장이 가능한 것 같아요!
https://docs.rs/rss/2.0.12/rss/struct.ItemBuilder.html#method.extensions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extension을 추가하려면 따로 schema 정의가 필요할 듯 해서, 일단 지금은 냅두고 최종 스펙 확정될 때 추가해보도록 하겠습니다. #19
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다 👍
tokio = { version = "1.44.1", features = ["full"] } | ||
rss = { version = "2.0", optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p5) rss
크레이트를 선택적인 의존성으로 만드신 이유가 궁금합니다!
GitHub Actions에서 cron job으로 실행할텐데 rss
크레이트는 항상 의존성으로 포함되어야 하지 않나 싶어서요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cargo의 Features 기능을 사용하기 위해 optional
값을 참으로 설정했어요.
Rust의 feature 기능은 하나의 crate에서 기능들을 분리할 수 있도록 제공해요. 저는 각 소스코드의 rss 기능에 #[cfg(feature = "rss")]
플래그를 붙여서 rss
기능이 포함될 경우에만 함수와 trait impl이 포함되도록 했어요.
하지만 rss 기능은 기본적으로 사용될 예정이기 때문에 rss
feature를 기본값으로 추가했고(10번째 줄 참고), rss
feature가 rss
crate를 요구하도록 설정했어요(11번째 줄 참고).
=> optional
으로 설정한 이유는 cargo에서 no-default-features = false
이여서 rss
feature가 제외되었을 경우 rss
crate를 제외할 수 있도록 하기 위함이고, default feature 인 rss
에서 rss
crate를 dep로 명시하고 있기 때문에 일반적으로 cargo add ssufid
를 하면 rss
dependency가 자동으로 설치됩니다.
src/core/rss.rs
Outdated
ItemBuilder::default() | ||
.title(value.title) | ||
.link(value.url) | ||
.pub_date(value.created_at.format(&Rfc2822).unwrap()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extensions()
메서드를 활용하면 확장이 가능한 것 같아요!
https://docs.rs/rss/2.0.12/rss/struct.ItemBuilder.html#method.extensions
#️⃣연관된 이슈
Resolves #15
📝작업 내용
rss
crate를 이용한 rss 변환 기능 구현💬리뷰 요구사항(선택)