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

クリッカブルURLの仕様変更に対するロールバック #1965

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

beru
Copy link
Contributor

@beru beru commented Jul 15, 2024

PR対象

  • アプリ(サクラエディタ本体)

カテゴリ

  • 不具合修正

PR の背景

#1915 に対応するPRです。

2021年7月にmergeされた下記のコミットをrevertする事で「ブラウズ」と「クリッカブルURL」の挙動を元に戻して関連付けされたアプリケーションで開くことが出来るようにしました。

9a0ed9f
f44540f
f902c3c
fdac3ca
581c19d
9169b13
8501185

仕様・動作説明

PR の影響範囲

テスト内容

関連 issue, PR

#1915
#1702
#1705
#1708

参考資料

beru added 2 commits July 15, 2024 22:14
This reverts commit 9a0ed9f.

Revert "ブラウズ機能からOpenWithBrowserするときのパラメータを修正"

This reverts commit f44540f.
…educe_shellExecute"

This reverts commit d0757ef, reversing
changes made to aad0b65.
@beru beru requested review from berryzplus, kengoide and suconbu July 15, 2024 13:47
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@AppVeyorBot
Copy link

Build sakura 1.0.4361 completed (commit a45b41ff70 by @beru)

@beru beru self-assigned this Jul 15, 2024
Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue側で結論出てないのでPR作成は時期尚早と思います。

@beru
Copy link
Contributor Author

beru commented Jul 18, 2024

issue側で結論出てないのでPR作成は時期尚早と思います。

#1915 で議論が今後行われるのかは良く分かりませんが、仮に議論が行われなくて何も対応がされないと、問題指摘があっても放置になるのでちょっとかわいそうではありますね。ただ自分がここで呟いても議論が進む事は無いでしょうが。。

今思うと #1702#1708 の変更ではヘルプに書かれている説明から外れた変更になってしまっているので(レビュー時にそこを指摘して)もし仕様変更するならヘルプの記載も同時に変えるようにするべきだったと思います。

#1702#1708 で加えた変更を設定で有効無効切り替えられるようにする事は可能ですが、ひとまず revert しても良いのかなと。

@berryzplus
Copy link
Contributor

結局「こまってない」なんですよね。

優先度は下げてよいと思います。

@beru
Copy link
Contributor Author

beru commented Jul 20, 2024

結局「こまってない」なんですよね。

私は困ってないです。takeyamajpさんとかは困っているというか仕様変更されて面倒くさくなっていると思いますが。

優先度は下げてよいと思います。

最近CrowdStrikeによって引き起こされたBSODに比べると緊急度は高くないと思います。

@yuksiy
Copy link

yuksiy commented Aug 10, 2024

初めまして。yuksiyと申します。

結局「こまってない」なんですよね。

私は困ってないです。takeyamajpさんとかは困っているというか仕様変更されて面倒くさくなっていると思いますが。

私は困っている者の1人です。
念のため、困っている内容を具体的に書かせて頂くと、
v2.4.1までは「sample.txt」ようなファイルをサクラエディタで開き、
画面中の任意フォルダ名の箇所をダブルクリックすると、エクスプローラで該当フォルダが開かれ、
任意ファイル名の箇所をダブルクリックすると、関連付けられた任意アプリで該当ファイルが開かれていましたが、
v2.4.2になってからはフォルダもファイルも開けなくなってしまったことです。

困ったと同時に、不便になったと感じたため、v2.4.2の使用を中止して、
v2.4.1に戻して現在まで使い続けている状況です。
ですので、本PRがマージされるとありがたいです。

ただ個人的には、一旦v2.4.1に戻すことによって困っていたことは回避できているので、
berryzplusさんやberuさんの書かれているように、優先度低めでよいと思います。

@berryzplus
Copy link
Contributor

可能なrevert戦略についてメモっておきます。

目的

変更前に戻す。

変更前とは aad0b65 のこと。
#1703 のマージコミット。)

revertイメージ

以下のコミットで適用された変更がすべて「変更前の状態」になったことを証明する。

image

対象ファイルこんだけ。
image

※この情報は本来、revertを要求した issue #1915 の作成者が用意すべき。

考慮事項

だいぶ古いPull Requestなので、revert対象ファイルに対して「その後の変更」が加えられている可能性あり。

このPRではおそらくそれも考慮済みだが、レビュアーが全部自分で確認しなおさないといけない。

で、実際に確認していくと、自分でPR作ったほうが早いくらいの作業量になる。

分類は次のような感じ。

  • このファイルは純粋revertなので、変更前と完全一致。
  • このファイルは後日別件で修正が入ったので、修正前とここだけ違う。
  • このファイルはrevertとは関係ないが、後日別件で入った修正を受けてここを変える必要がある。

まとめると「必要な情報が足りないのでレビューできない」になるんだけど
不足情報を @beru さんに要求するのは「筋違い」のような。

「俺ならできるぜ」みたいなノリになれる人が居れば、レビュー敢行を否定するものではない。

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.

4 participants