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

Allow hyphens in project name #1376

Merged
merged 3 commits into from
Nov 10, 2014
Merged

Conversation

invisibleroads
Copy link
Contributor

Convert hyphens in project_name to underscores in pkg_name

Convert hyphens in project_name to underscores in pkg_name
@mmerickel
Copy link
Member

+1 LGTM

@invisibleroads
Copy link
Contributor Author

Would hyphens in the project name be a safe change? @mcdonc @tseaver

@tseaver
Copy link
Member

tseaver commented Jul 24, 2014

Two things:

  • Should we be using some API from setuptools for matching / generating package name from project name?
  • Either way, we need a test showing that the conversion works.

@invisibleroads
Copy link
Contributor Author

@tseaver, I added a test to make sure that hyphens in the project name are properly converted into underscores in the package name.

I looked at setuptools and found setuptools.dist.check_packages(), which validates package names using the regular expression \w+(\.\w+)*. However, that regular expression permits hyphens in the middle of the package name, which doesn't help. Interestingly, I found it is possible to import invalid package names using __import__('invalid-package-name'). For these reasons, I decided not to use functions from setuptools to generate the package name.

@invisibleroads
Copy link
Contributor Author

Hi Michael, Hi Tres,
Do you have any other feedback regarding these suggested changes?
RHH

@mmerickel @tseaver

@tseaver
Copy link
Member

tseaver commented Nov 9, 2014

Thanks for backing out the extraneous changes via c1ef71c. This change looks good to me.

mmerickel added a commit that referenced this pull request Nov 10, 2014
@mmerickel mmerickel merged commit 726bbe8 into Pylons:master Nov 10, 2014
@mmerickel
Copy link
Member

Thanks again!

mmerickel added a commit that referenced this pull request Nov 10, 2014
@invisibleroads
Copy link
Contributor Author

Thank you for maintaining this great descendant of Pylons.

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