Disallow gem names with CAPS for users AND TESTS #451

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
@daveott

daveott commented Aug 22, 2012

Fixes #421. Adds uppercase letters to list of invalid characters on rubygem creation.

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 22, 2012

This pull request passes (merged f0e664f into 29fabb2).

This pull request passes (merged f0e664f into 29fabb2).

@cmeiklejohn

This comment has been minimized.

Show comment Hide comment
@cmeiklejohn

cmeiklejohn Sep 8, 2012

Contributor

Ah, unless I'm missing something here it looks like that name validator is run on all saves, not just creates so this would cause gems with capitals already in the system to be invalid suddenly. Also, what is the motivation behind this change?

Contributor

cmeiklejohn commented Sep 8, 2012

Ah, unless I'm missing something here it looks like that name validator is run on all saves, not just creates so this would cause gems with capitals already in the system to be invalid suddenly. Also, what is the motivation behind this change?

@cmeiklejohn

This comment has been minimized.

Show comment Hide comment
@cmeiklejohn

cmeiklejohn Sep 8, 2012

Contributor

Ah, derp. I just noticed the original issue. I think this still needs to be patched to not cause existing gems to be invalid, and a test case which verifies that case would be great. Thanks for the pull!

Contributor

cmeiklejohn commented Sep 8, 2012

Ah, derp. I just noticed the original issue. I think this still needs to be patched to not cause existing gems to be invalid, and a test case which verifies that case would be great. Thanks for the pull!

@coffeencoke

This comment has been minimized.

Show comment Hide comment
@coffeencoke

coffeencoke Nov 4, 2012

working on this.

working on this.

@coffeencoke

This comment has been minimized.

Show comment Hide comment
@coffeencoke

coffeencoke Nov 4, 2012

Added #481 - looks like the implementation was already there, just needed to add a unit test.

Added #481 - looks like the implementation was already there, just needed to add a unit test.

@evanphx

This comment has been minimized.

Show comment Hide comment
@evanphx

evanphx Jul 12, 2013

Owner

I'm going to go ahead and say no to this (8 months in, I know). We need to case preserve but be case insensitive in checks.

Owner

evanphx commented Jul 12, 2013

I'm going to go ahead and say no to this (8 months in, I know). We need to case preserve but be case insensitive in checks.

@evanphx evanphx closed this Jul 12, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment