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

自動予約登録一覧で、「削除(予約ごと削除)」をボタンとコンテキストメニューに追加 #1

Closed
wants to merge 1 commit into from

Conversation

tkntrec
Copy link

@tkntrec tkntrec commented Apr 23, 2014

「2ch_EDCB/36/799」の名前でコードを取り込んでもらったものです。
先日はありがとうございました。

少々迷ったのですが今回1回だけプルリクエストさせていただきます。
もし使えそうならお願いします。

それから、リクエストメッセージの中でこのような話題はそぐわないかもなのですが、今後はプルリクエストはせずに、xtne6fさんの最新コミットからはNetwok図で矢印を伸ばさないような形で併走させてもらおうと考えているのですが、お邪魔でしょうか。

プルリクエストについては、現在のEDCBの状況では、私以外に「xtne6fさんverのforkに、私の使っているパッチの機能が欲しい」という人がいれば、その人がアクションを起こす(prとか2ch書込とか)という理解で良いのですよね?

@xtne6f
Copy link
Owner

xtne6f commented Apr 23, 2014

ありがとうございます。頂戴します。
ただ、このままだとマージできない事情(下図)があるので可能であればこちらのworkまたはmisc-uiから派生する形で、misc-uiのほうにリクエストしてください。困難であればcherry-pickで採らせて頂く形でも構いません。
graph

ブランチの進め方は自由です。当方もepgdatacapbon氏のフォークですし。
もちろん邪魔なんてこともないです。
個々のパッチ提供については、自分の趣味にあうかどうか次第なので何とも言えませんが
(割と保守派なので原作10.66~10.69との互換を失うようなパッチは拒否するかもしれません)
あえてリクエストせずとも各自でwork-plusから派生したフォークを公開すれば済む話で、気楽にやればいいように思います。

以下細かいことです:
これ↓はいらないような気がします

if (info.KeyEnabled == "はい")

EpgAutoDataItemの項目を弄ったときに破綻するので、いらないなら無いほうが安全です
最終行の「自動予約登録項目自体の削除」は予約削除の手前のほうがいいかもしれません。削除中に再予約される可能性があります
あと、かなり些末ですが「削除(予約ごと削除)」はもう「予約ごと削除」と言い切っちゃってもニュアンス伝わるように思いますがどうでしょう?

@tkntrec
Copy link
Author

tkntrec commented Apr 24, 2014

ご指摘、アドバイスありがとうございます。
改めてmisc-uiにprします。

「削除(予約ごと削除)」はもう「予約ごと削除」
その通りですね。そうします。

if (info.KeyEnabled == "はい")
ここは作成時少し悩みました。
無効の項目は一覧表示から対象数(予約数)が見えないため安全側にしていたのですが、ダイアログ出しているのだから、確かに無くてもよさそうです。
(条件を入れる場合でも、andKeyから直接判定すべきでしたね。)

「自動予約登録項目自体の削除」は予約削除の手前のほうがいいかも
削除中に再予約される可能性
その方が良いですね。
この場合も「自動予約登録項目削除(成功)→予約削除(失敗)」のときは割と嫌な状態にはなりますが、エラーは明示的であって、現在の「気付かず再予約されてしまう」ほど致命的ではないわけですね。

@tkntrec
Copy link
Author

tkntrec commented Apr 24, 2014

一つ前のコメント、引用符使ったら妙な具合になってしまいました。
読みにくくて申し訳ない。

@tkntrec
Copy link
Author

tkntrec commented Apr 24, 2014

あと、ブランチの件は、今回のプルリクエスト完了後にちょっと整理します。

@xtne6f xtne6f closed this Apr 24, 2014
@tkntrec tkntrec deleted the my_pr branch April 25, 2014 10:01
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