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

chm のビルドでリトライを実装 #622

Merged
merged 3 commits into from
Nov 18, 2018

Conversation

m-tmatma
Copy link
Member

chm のビルドでリトライを実装
#584 のワークアラウンド

@m-tmatma
Copy link
Member Author

ビルド失敗は発生しなかったのでワークアラウンドが機能しているかはわかりませんが、
失敗しない場合の悪影響はないと思います。

レビューお願いします。

build-chm.bat Outdated Show resolved Hide resolved
build-chm.bat Outdated Show resolved Hide resolved
@m-tmatma
Copy link
Member Author

修正しました。

@ds14050
Copy link
Contributor

ds14050 commented Nov 18, 2018

そこまでパラメータ化したなら

for %%B in (
create-project.bat
build-project.bat
) do (
call :ExecBat "%~dp0%%~B" %PLATFORM% %CONFIGURATION% ^
|| (echo ERROR %%~B %ERRORLEVEL% & exit /b 1)
)

sakura/build-all.bat

Lines 32 to 54 in deb2fb9

if "%CONFIGURATION%" == "Release" (
set BatchJobs=build-sln.bat^
build-chm.bat^
build-installer.bat^
zipArtifacts.bat
) else (
set BatchJobs=build-sln.bat^
externals\cppcheck\install-cppcheck.bat^
run-cppcheck.bat^
zipArtifacts.bat
)
if "%PLATFORM%" == "MinGW" (
set BatchJobs=build-gnu.bat
rem Skip all other batch files because they reject MinGW platform.
)
rem run
for %%B in (%BatchJobs%) do (
call :ExecBat "%~dp0%%~B" %PLATFORM% %CONFIGURATION% ^
|| (echo error %%~B & exit /b 1)
)
exit /b 0
でやって見せたようにコードを共通化してもいいと思うのですが、したくない理由があるのですか?

@m-tmatma
Copy link
Member Author

これはワークアラウンドなのであまりコードを変えたくないからです。
バグってたので、バグりにくくしただけです。

@ds14050
Copy link
Contributor

ds14050 commented Nov 18, 2018

しかしコピペミスにより(というよりコピペしなかったことにより)すでにバグが入ってしまっていました。多く書けば書くほどミスが入り込む余地が増えます。一度だけ書いてそれを完璧に仕上げる方がバグに対処しやすいと考えますが、リファクタリングに反対する理由がないのであればとりあえずは満足です。

追記。「バグ」の意味を違えてしまっていたせいであまり関連性のないコメントになっていました。m-tmatma さんはおそらく AppVeyor で不定期に発生する事象をバグと呼んでいました。

@m-tmatma m-tmatma merged commit ace82c2 into sakura-editor:master Nov 18, 2018
@m-tmatma m-tmatma deleted the feature/retry-hhc branch November 18, 2018 09:38
@m-tmatma
Copy link
Member Author

しかしコピペミスにより(というよりコピペしなかったことにより)すでにバグが入ってしまっていました。

都合により、PC の再インストールしたもので、notepad で作業してコピペミスしました。
ただコミット時にちゃんと確認できていませんでした。
winmerge とかインストールしてなかったので確認が甘かったです。
GitHub の画面とかでも確認できたんだけど。

@ds14050 ds14050 assigned ds14050 and unassigned ds14050 Nov 18, 2018
@ds14050
Copy link
Contributor

ds14050 commented Nov 18, 2018

あまり関連性のないコメント

なのであまり気になさらないように。

HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI appveyor など CI 関連 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants