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

Add title to module document for Docs target #131

Merged
merged 3 commits into from Apr 12, 2017

Conversation

@AiOO
Copy link
Collaborator

@AiOO AiOO commented Mar 31, 2017

related #125

@AiOO AiOO self-assigned this Mar 31, 2017
@AiOO AiOO requested review from dahlia and kanghyojun Mar 31, 2017
@@ -99,6 +100,8 @@ $case expr'
module' :: BoundModule Docs -> Html
module' docsModule = layout pkg path $ [shamlet|
<h1><code>#{path}</code>
$maybe tit <- title
<p>#{tit}

This comment has been minimized.

@dahlia

dahlia Mar 31, 2017
Member

구두로도 얘기했지만, <h1> 뒤쪽에 나오면 좋을 것 같습니다.

title =
case mod' of
Just mod'' -> moduleTitle mod''
_ -> Nothing

This comment has been minimized.

@dahlia

dahlia Mar 31, 2017
Member

아마 hlint가 이미 오류를 냈을지도 모르겠는데… case … of … 표현식은 fmap moduleTitle mod' 정도로 줄일 수 있을 겁니다.

Prelude> fmap (+1) (Just 123)
Just 124
Prelude> fmap (+1) Nothing
Nothing
title :: Maybe Html
title =
case mod' of
Just mod'' -> moduleTitle mod''

This comment has been minimized.

@kanghyojun

kanghyojun Apr 1, 2017
Member

ㅋㅋㅋ 차라리 foundModule 같은 이름은 어떨까요?

This comment has been minimized.

@dahlia

dahlia Apr 1, 2017
Member

@admire93 결과가 모듈 제목이라 지금 이름이 더 나은 것 같읍니다.

This comment has been minimized.

@AiOO

AiOO Apr 12, 2017
Author Collaborator

reject

Copy link
Member

@kanghyojun kanghyojun left a comment

소소하지만 코멘트하나했읍니다

@AiOO AiOO mentioned this pull request Apr 12, 2017
13 of 13 tasks complete
@dahlia
dahlia approved these changes Apr 12, 2017
@AiOO AiOO merged commit f656b78 into nirum-lang:master Apr 12, 2017
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants