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

Allow review_version to be nil, which means that I don't care about the version #592

Merged
merged 3 commits into from
Apr 25, 2016

Conversation

kdmsnr
Copy link
Collaborator

@kdmsnr kdmsnr commented Apr 24, 2016

バージョンを書くのが面倒な人も許容したい

@takahashim
Copy link
Collaborator

cf. #276

@kdmsnr
Copy link
Collaborator Author

kdmsnr commented Apr 24, 2016

気にする人は書けばいいかと

@takahashim
Copy link
Collaborator

エラーにするのがいいのかはまだ迷っているのですが、マークアップの意味が変わって文書が期待と異なる出力になるかもしれない、というのは大抵のひとは「気にする」のではないかなあ、という気持ちもあるのでした。

@kdmsnr
Copy link
Collaborator Author

kdmsnr commented Apr 24, 2016

warningは注意勧告なので、自分で消せるようにしておきたいです。で、その方法は何であっても構わないんですが、「config.ymlにバージョンを書かない」が手軽なのかなあと。

@kdmsnr
Copy link
Collaborator Author

kdmsnr commented Apr 24, 2016

ああ、やりたいことがだんだんわかってきました(今さら)。これは、無事にRe:VIEWのバージョンアップに追従したい人用なんですよね。で、そういう人はマメだから、こういう機能があってもいいと思います。

一方で、単に執筆環境を壊したくない人は、Bundlerでバージョンを固定すればいいだけなので、そもそもreview_versionを設定したくはないです。

@takahashim
Copy link
Collaborator

そうですそうです>無事にRe:VIEWのバージョンアップに追従したい人用

そして、「単に執筆環境を壊したくない人は、Bundlerでバージョンを固定すればいいだけ」なので、2.0で何があってもバージョンアップしないから既存のコードが動かなくても困らないのでは…という気持ちもありました。

が、2.x以降でも気にしない人向けのケアが必要だ、というのは確かに需要ありそうなので、例えばそういう人向けには「review_versionにnilとかfalseとか0とか入れれば無視する」という設計にするのはどうでしょうか。

@kdmsnr
Copy link
Collaborator Author

kdmsnr commented Apr 24, 2016

self["review_version"].blank? の場合は無視するという意味では、このPRがいちおうそういう挙動にはなってますー。

@takahashim
Copy link
Collaborator

config.ymlにreview_versionの項目を追加してない(1.xのconfig.ymlからそのまま使おうとした)人はエラーにしたいのでした…。
なので@config.key?("review_version")が偽の場合にはエラーにするのを追加する感じです。真の場合は上記のpull req.のコードの挙動でよいかと。

@kdmsnr
Copy link
Collaborator Author

kdmsnr commented Apr 25, 2016

更新しました

@kdmsnr kdmsnr added this to the 2.0.0 milestone Apr 25, 2016
@takahashim
Copy link
Collaborator

更新どうもです!

@takahashim takahashim merged commit 1713303 into kmuto:master Apr 25, 2016
@kdmsnr kdmsnr deleted the i-dont-care-review-version branch April 25, 2016 03:39
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.

2 participants