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

scripts: script update must be approved #6

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

Conversation

Spiderpowa
Copy link
Contributor

@Spiderpowa Spiderpowa commented Oct 1, 2019

Several changes:

  1. No need to upload run_bp.py.sha1 file.
  2. The script will check if the commit it verified.
  3. Add approval flow

@Spiderpowa Spiderpowa requested a review from aitjcize October 1, 2019 17:57
@Spiderpowa Spiderpowa changed the title script: script update must be approved scripts: script update must be approved Oct 1, 2019
@Spiderpowa Spiderpowa changed the base branch from w-script to master October 2, 2019 04:23
@Spiderpowa Spiderpowa changed the base branch from master to w-script October 2, 2019 04:23
@Spiderpowa Spiderpowa changed the base branch from w-script to master October 2, 2019 04:43
_SCRIPT_APPROVE_PATH_TMPL = _SCRIPT_PATH + '.%s'
_SCRIPT_APPROVE_SRC_TMPL = ('https://raw.githubusercontent.com/'
'%s/%s/%%s/%s' % (_SCRIPT_ORG, _SCRIPT_REPO, _SCRIPT_APPROVE_PATH_TMPL))
_SCRIPT_APPROVER = ['aitjcize', 'popodidi', 'JM00oo', 'Spiderpowa']
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove popodidi ...

@@ -155,20 +165,79 @@ def check_environment():
'system time')


def github_get_commits(path):
return '%s/repos/%s/%s/commits?path=%s&sha=%s' % (_GITHUB_API, _SCRIPT_ORG, _SCRIPT_REPO, path, _SCRIPT_BRANCH)
Copy link
Contributor

Choose a reason for hiding this comment

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

please check 80 line alignment for all changes

Copy link
Contributor

Choose a reason for hiding this comment

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

also the sha=%s 's argument is _SCRIPT_BRANCH?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sha parameter takes branch as well.

aitjcize
aitjcize previously approved these changes Oct 3, 2019


def github_get_approved_commit(commit, approver):
with urllib.request.urlopen(github_get_commits(_SCRIPT_APPROVE_PATH_TMPL % approver),
Copy link
Contributor

Choose a reason for hiding this comment

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

won't this take a long time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's pretty fast

@@ -155,20 +165,79 @@ def check_environment():
'system time')


def github_get_commits(path):
return '%s/repos/%s/%s/commits?path=%s&sha=%s' % (_GITHUB_API, _SCRIPT_ORG, _SCRIPT_REPO, path, _SCRIPT_BRANCH)
Copy link
Contributor

Choose a reason for hiding this comment

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

also the sha=%s 's argument is _SCRIPT_BRANCH?

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