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

TEMP_GIT_SHORT_COMMIT_HASH および TEMP_GIT_COMMIT_HASH を廃止する #1193

Merged

Conversation

m-tmatma
Copy link
Member

@m-tmatma m-tmatma commented Feb 11, 2020

PR の目的

TEMP_GIT_SHORT_COMMIT_HASH および TEMP_GIT_COMMIT_HASH を廃止する

カテゴリ

  • ローカルビルド
  • CI関連
    • Appveyor
    • Azure Pipelines

PR の背景

以下の一連の対応により、 APPVEYOR 依存変数を使わなくなった。
#1183
#1184
#1185
#1186
#1187

APPVEYOR 依存変数を削除する対応の中で、#886 (comment) の際に既存動作を変えないために #969 で TEMP_GIT_COMMIT_HASH と TEMP_GIT_SHORT_COMMIT_HASH を導入した。

#1183 (comment)
#1190 (comment)

  • TEMP_GIT_SHORT_COMMIT_HASH : 8文字。なんで?
  • TEMP_GIT_COMMIT_HASH フル桁と同じものな気がする。
  • TEMP_GIT_XXXってなんでしたっけ?w

の一環で TEMP_GIT_SHORT_COMMIT_HASH および TEMP_GIT_COMMIT_HASH を廃止する

PR のメリット

変数の定義が減るのでシンプルになる。

PR のデメリット (トレードオフとかあれば)

デメリットではないが、zip アーカイブを作成するときに SHORTHASH という変数の定義を
GIT_SHORT_COMMIT_HASH の変数に置き換えたので、ローカルビルドしたときに
zip ファイル名にコミットハッシュの短縮形が入る。(現状はローカルビルドの場合は入っていない)

↑ もともと TEMP_GIT_SHORT_COMMIT_HASH および TEMP_GIT_COMMIT_HASH は
APPVEYOR_REPO_COMMIT および APPVEYOR_SHORTHASH の廃止の際に仕様が変わるのを
防ぐためのものだった。

※ この PR は CI ビルドの場合は影響はない。

PR の影響範囲

関連チケット

#886
#969
#1183
#1184
#1185
#1186
#1187

参考資料

1. GIT_SHORT_COMMIT_HASH => GITHUB_COMMIT_URL
2. GITHUB_PR_NUMBER_LABEL => GITHUB_COMMIT_URL_PR_HEAD

1 はもともと TEMP_GIT_SHORT_COMMIT_HASH だったので CI の場合に有効になる変数だったが、
GIT_SHORT_COMMIT_HASH に変更するとローカルビルドでも有効になるので動作を合わせるために
GITHUB_COMMIT_URL で判断するように変更

2 は変更の必要はないが、 1 と処理を同様にするために合わせて変更する。
@m-tmatma m-tmatma added this to the v2.4.0 milestone Feb 11, 2020
@m-tmatma m-tmatma added appveyor azure pipelines CI appveyor など CI 関連 【ChangeLog除外】 local build labels Feb 11, 2020
@AppVeyorBot
Copy link

Build sakura 1.0.2618 completed (commit 2a679366fd by @m-tmatma)

@berryzplus
Copy link
Contributor

問題なさそうに見えます。

CIビルドの成果物 でリンク先が適切に設定されているか見て見ました。

一連のやり取りで仕様理解を深めたアタマで見ると、今回の変更分じゃないとこばっかし気になった感じです。

バージョン情報の表示テキスト

サクラエディタ   v2.4.0.2618 32bit dev
(GitHash 2a679366fd5db92831da83e66fcdd26d544d3b90)
(GitURL https://github.com/sakura-editor/sakura.git)

      Compile Info: V1916 WPR WIN601/I800/C000/N601
      Last Modified: 2020/2/11 09:09:05

気になった点(全力で別件)

  • バージョン情報の2行目にあるハッシュにリンクが付いてない。
  • バージョン情報の2行目にあるGitHub URLにリンクが付いてない。
  • PR 1193 のリンク先は 813adce になっている。
  • SUPER 別件: Build URL のところ、「Built by Appveyor: 1.0.2618」とかしたらカッコいい気がしました。

@m-tmatma m-tmatma merged commit fc8f914 into sakura-editor:master Feb 11, 2020
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 16, 2020
…EMP_GIT_COMMIT_HASH

TEMP_GIT_SHORT_COMMIT_HASH  および  TEMP_GIT_COMMIT_HASH を廃止する
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appveyor azure pipelines CI appveyor など CI 関連 【ChangeLog除外】 local build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants