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

Jump to bottom

setup.py needs cleanup #43

Closed
atagar opened this issue Dec 21, 2019 · 5 comments
Closed

setup.py needs cleanup #43

atagar opened this issue Dec 21, 2019 · 5 comments

Comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
@atagar @msramalho
@atagar
Copy link
Contributor

@atagar atagar commented Dec 21, 2019

Migrated from ticket 31234.

  1. We should avoid importing our own modules as this prevents us from being installed from other directories.
  2. Optionally use setuptools so bdist_wheel can be available.
@msramalho msramalho mentioned this issue Mar 30, 2020
Merged
@msramalho
Copy link
Contributor

@msramalho msramalho commented Mar 30, 2020 •
edited

Hey @atagar, I will be trying to fix this.

I have found a very interesting page that gives several options to solve this. Can you please take a look and check which would make more sense?

In the meantime, I have migrate distutils to setuptools in #58

@msramalho
Copy link
Contributor

@msramalho msramalho commented Mar 30, 2020 •
edited

By the way, the original ticket mentioned:

  • All the large strings should be in separate files.
    Should this also be fixed? if so, what's the criteria to use when telling which strings to isolate to a file?

torproject-pusher pushed a commit that referenced this issue Apr 2, 2020
@atagar
Migrate from distutils to setuptools
2c043f4 
Setuptools doesn't provide anything we need so I stuck with distutils, but
that's discouraged nowadays. Thanks to Miguel finally making the move.

  #43
@atagar
Copy link
Contributor Author

@atagar atagar commented Apr 2, 2020

Thanks Miguel! Pushed along with some test fixes.

As for getting the package version thanks for pointing that out! We actually previously did the first option, so we could probably simply revert this to do that.

@msramalho msramalho mentioned this issue Apr 2, 2020
Closed
@msramalho
Copy link
Contributor

@msramalho msramalho commented Apr 2, 2020

Hey @atagar, thanks for the tips, I have adapted the previous code and submitted another PR.

@atagar
Copy link
Contributor Author

@atagar atagar commented Apr 3, 2020

Thanks Miguel! Merged.

@atagar atagar closed this Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment