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

Script to release ok-client #288

Merged
merged 24 commits into from
Mar 4, 2017
Merged

Script to release ok-client #288

merged 24 commits into from
Mar 4, 2017

Conversation

knrafto
Copy link
Contributor

@knrafto knrafto commented Mar 1, 2017

Resolves #264, #276.

Test plan:

  • Clone into a new github repo for testing
  • Comment out the python setup.py sdist upload
  • Edit GITHUB_REPO to point to the testing repo
  • Edit OK_SERVER_URL to point to localhost:5000
  • Try to "release" the fake client. Should see
    • Version bump pushed to github
    • GitHub release with correct binary
    • Updated client version on localhost:5000

@knrafto
Copy link
Contributor Author

knrafto commented Mar 4, 2017

It works https://github.com/okpy/ok-client/releases/tag/v1.10.3

Feel free to try it out there. You'll need to have the server running locally.

Copy link
Member

@Sumukh Sumukh left a comment

Choose a reason for hiding this comment

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

I haven't tried it but it looks good. I'll try it out soon

contains okpy's Pypi credentials.
3. From the base of the repo, make sure your virtualenv is activated and run
* Your virtualenv is activated and you are on the master branch.
* Your `~/.pypirc` contains okpy's PyPI credentials.
Copy link
Member

@Sumukh Sumukh Mar 4, 2017

Choose a reason for hiding this comment

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

If this is supposed to be a checklist - something about double checking that tests have passed would probably be good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests only take a few seconds, so I'll just add that to the script's sanity checks.

README.md Outdated
3. From the base of the repo, make sure your virtualenv is activated and run
* Your virtualenv is activated and you are on the master branch.
* Your `~/.pypirc` contains okpy's PyPI credentials.
* A file `.github-token` contains a [GitHub access token](https://help.github.com/articles/creating-an-access-token-for-command-line-use/).
Copy link
Member

Choose a reason for hiding this comment

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

What scopes/permissions does the token need? (It's fine to just say select all of them)

)

print('Updating version on {}...'.format(OK_SERVER_URL))
# Create a fake assignment to log in. I'm not happy about this
Copy link
Member

Choose a reason for hiding this comment

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

Out of scope for this PR: Getting a token seems like the one thing we should be able to support without an assignment. Maybe we should add a method somewhere that accepts all of the default values.

I've opened up a URL in other scripts - but I don't recommend it for this file.

def get_token():
    print("Get an access token for your OK account & paste it below")
    webbrowser.open('https://ok-oauth.app.cs61a.org/')
    return input("Your OK Token: ").strip()

release.py Outdated
try:
output = subprocess.run(command, **kwargs)
except subprocess.CalledProcessError as e:
print(str(e))
Copy link
Member

Choose a reason for hiding this comment

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

Curious: Any reason this isn't going into sys.stderr? (I'm ambivalent about where it goes)

@Sumukh
Copy link
Member

Sumukh commented Mar 4, 2017

Being able to use python (or python3) release.py directly would be nice so that isn't as much of an issue for other people. Currently we can't because

  File "release.py", line 73, in <module>
    os.chdir(os.path.dirname(__file__))
FileNotFoundError: [Errno 2] No such file or directory: ''

I actually haven't been able to get the script smoothly because of the python issues (and the virtualenv issues). I did get to upload a release though.

  • Sanity check to keep the version number starting with v1. instead of just v
    If we want to change the major version I think it's reasonable to change the release script. It would be super easy for me to accidentally release v11.02 instead of v1.10.2

@Sumukh
Copy link
Member

Sumukh commented Mar 4, 2017

Also I think there's a dependence on running python3 setup.py develop to get the ok-publish command going if it wasn't before.

From the diff it seems like that part hasn't been added?

@knrafto
Copy link
Contributor Author

knrafto commented Mar 4, 2017

This does use the virtualenv python3. usr/bin/env python3 invokes the first executable named python3 on your PATH, which will be the virtualenv python3 if you've activated it. I've fixed running just python3 release.py though.

I don't want to hardcode v1., since it's conceivable that we'll want to release version 2 sometime. How about the major version number should not increase by more than 1?

@Sumukh
Copy link
Member

Sumukh commented Mar 4, 2017

I don't want to hardcode v1., since it's conceivable that we'll want to release version 2 sometime. How about the major version number should not increase by more than 1?

Sure.

@knrafto
Copy link
Contributor Author

knrafto commented Mar 4, 2017

I think I've addressed everything. Any other concerns before I merge?

@Sumukh
Copy link
Member

Sumukh commented Mar 4, 2017

Nope - feel free to merge & then release with this script :)

@knrafto knrafto merged commit fcf2877 into master Mar 4, 2017
@knrafto knrafto deleted the release branch March 4, 2017 23:43
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