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

implementation. was able to register many jobs. #3

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

implementation. was able to register many jobs. #3

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 6, 2013

"/"区切りの複数のジョブを登録できるようにします。
jenkinsのジョブ名に"/"は使えないので"/"を区切り文字にしました。

@mallowlabs
Copy link
Owner

いくつか var の宣言が漏れていてグローバル変数になっている変数があります。
具体的には i, l, jobs, jobName です。
グローバル変数の使用は好ましくないので修正できませんか?

@mallowlabs
Copy link
Owner

それと(どれだけいるかはわかりませんが)既存ユーザがアップデートをすると急に通知されなくなるので
localStorage["job-names"] が設定されていない かつ localStorage["job-name"] が設定されている場合は、
古い設定を使うようにできませんか?
面倒であれば localStorage["job-name"] に複数ジョブ名を入れる設計でもかまいません。

@ghost
Copy link
Author

ghost commented Mar 12, 2013

こちらも修正いたしました。
確認いただいて問題無ければマージよろしくお願いします。
細かい修正が多く申し訳ありません。

@mallowlabs
Copy link
Owner

こちらこそ細かい指摘が多くて申し訳ないです…。

テストしていて気づいたのですが、複数ジョブが指定された場合に prevBuild のチェックがうまく動いてないようです。複数ジョブを指定した場合にも、最新のビルドのみが通知されて欲しいのですが、修正は簡単そうでしょうか?

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