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

pip install cirrus #59

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

Conversation

yangryan0
Copy link
Collaborator

No description provided.

@jcarreira
Copy link
Owner

jcarreira commented Jul 14, 2018

I think we should rename all the files with spaces in the name (and use underscores instead). The changes seem reasonable. @andrewmzhang Can you also check this?

@@ -0,0 +1,19 @@
Copyright (c) 2018 The Python Packaging Authority
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might want to make this the Apache License, to match the License of cirrus (c++)

setuptools.setup(
name="cirrus",
version="0.0.2",
author="jcarreira",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think by convection this is the author's name, not username.

I actually wrote a version of setup.py a while back, but never submitted a proper PR for it. Pick and choose what you need from here.

https://github.com/andrewmzhang/cirrus-1/blob/cirrus_python/python/frontend/cirrus/setup.py

you should probably take the first 5 lines from my file as well as license=open('LICENSE') and the packages=find_packates(exclude=('test', 'docs')). Everything else is good.

packages=setuptools.find_packages(),
classifiers=(
"Programming Language :: C++",
"License :: OSI Approved :: MIT License",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably should be Apache (mentioned above)

@jcarreira
Copy link
Owner

We should bring this PR up to date because some things here were added in a recent PR from @andrewmzhang (namely LICENSE, setup,py, ..).

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.

3 participants