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

すべてを含んだ zip を作成しないようにする (#514 の軽減策) #587

Merged
merged 2 commits into from
Oct 28, 2018

Conversation

m-tmatma
Copy link
Member

すべてを含んだ zip を作成しないようにする (#514 の軽減策)

@m-tmatma m-tmatma added the CI appveyor など CI 関連 【ChangeLog除外】 label Oct 27, 2018
@m-tmatma m-tmatma added this to the next release milestone Oct 27, 2018
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.

allのzipを作らないようにする、で賛成です。

今後恒久的に作らないようにするか、一時的に止めるかがちょっと分かってないです。
個人的には一時的に止める、の意図でした。
いつになるか分かりませんが、GitHub Releaseにはallがあったほうがいいような気がしています。

@@ -91,7 +91,6 @@ More information: https://github.com/sakura-editor/sakura/issues/6
5. 自分がダウンロードしたいものをクリックしてダウンロードします。
- (ユーザー用) 末尾に `Exe` がついてるのが実行ファイルのセットです。
- (ユーザー用) 末尾に `Installer` がついてるのがインストーラのセットです。
- (すべて欲しい人向け) `All` がついてるのがバイナリ、インストーラ、ビルドログ、アセンブラ出力のフルセットです。
Copy link
Contributor

Choose a reason for hiding this comment

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

allを作らないのは暫定じゃないんですか?

他の人の意見もあると思いますが、「一時停止中」で残しておいていいような気がします。

zipArtifacts.bat Outdated
@@ -146,16 +146,12 @@ set WORKDIR_LOG=%WORKDIR%\Log
set WORKDIR_EXE=%WORKDIR%\EXE
set WORKDIR_INST=%WORKDIR%\Installer
set WORKDIR_ASM=%BASENAME%-Asm
set OUTFILE=%BASENAME%-All.zip
Copy link
Contributor

Choose a reason for hiding this comment

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

この定義を残したままでも all を出力しない、は実現できると思います。

zipArtifacts.bat Outdated
set OUTFILE_LOG=%BASENAME%-Log.zip
set OUTFILE_ASM=%BASENAME%-Asm.zip
set OUTFILE_INST=%BASENAME%-Installer.zip
set OUTFILE_EXE=%BASENAME%-Exe.zip

@rem cleanup for local testing
if exist "%OUTFILE%" (
del %OUTFILE%
)
Copy link
Contributor

Choose a reason for hiding this comment

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

この定義を(ry
上とおんなじです。

zipArtifacts.bat Outdated
call %ZIP_CMD% %OUTFILE_LOG% %WORKDIR_LOG%

@rem copy text files for warning after zipping %OUTFILE% because %WORKDIR% is the parent directory of %WORKDIR_EXE% and %WORKDIR_INST%.
Copy link
Contributor

Choose a reason for hiding this comment

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

フォルダ構造を変えるわけではないのでコメントは残しておいていい気がします。

zipArtifacts.bat Outdated
@@ -242,11 +238,8 @@ copy /Y installer\warning.txt %WORKDIR%\
if "%ALPHA%" == "1" (
copy /Y installer\warning-alpha.txt %WORKDIR%\
)
call %ZIP_CMD% %OUTFILE% %WORKDIR%

Copy link
Contributor

Choose a reason for hiding this comment

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

アーティファクトを作っているのはこの一文なので、
ここをコメントアウトすればallは回収されないと思います。

枠組みとして残しておいて、GitHub Releaseへの登録のときだけ使う、みたいなことできないかな、と思いました。

@k-takata
Copy link
Member

appveyor.yml でアップロードするartifactsを限定するやり方もあります。

@m-tmatma m-tmatma force-pushed the feature/exclude-all-artifacts branch 3 times, most recently from 39674f5 to 7320433 Compare October 28, 2018 08:16
@m-tmatma m-tmatma force-pushed the feature/exclude-all-artifacts branch from 7320433 to 11fdce2 Compare October 28, 2018 08:20
@m-tmatma
Copy link
Member Author

修正しました。

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.

対応ありがとうございます。

当面はこれでいきましょう。

@m-tmatma m-tmatma merged commit 353584c into sakura-editor:master Oct 28, 2018
@m-tmatma m-tmatma deleted the feature/exclude-all-artifacts branch October 28, 2018 09:44
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
…l-artifacts

すべてを含んだ zip を作成しないようにする (sakura-editor#514 の軽減策)
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.

3 participants