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

Use shell quotes for the current platform on the commit hint after re-rendering #408

Merged

Conversation

nicoddemus
Copy link
Member

As discussed in #407

@nicoddemus
Copy link
Member Author

The Travis failure seems to be the same as in #407:

AttributeError: 'module' object has no attribute 'ci_register'

print(
"You can commit the changes with:\n\n"
" git commit -m 'MNT: Re-rendered with conda-smithy %s'\n" % __version__
" git commit -m {q}MNT: Re-rendered with conda-smithy {version}{q}\n"
Copy link
Member

Choose a reason for hiding this comment

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

Honestly I'd be ok just using " if that is better for Windows. Both should work fine on Unix as there is nothing fancy going on in the message.

Copy link
Member

@jakirkham jakirkham Jan 4, 2017

Choose a reason for hiding this comment

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

I know this was your contribution originally @minrk . Do you care if we just use " or was there some special reason for ' that is not coming to me now?

Copy link
Member

Choose a reason for hiding this comment

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

I only used ' because the surrounding string is ", so I wouldn't have to escape. No deeper reason than that. Switching the inner and outer quotes should work fine, too, or escape the inner quotes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @jakirkham and @minrk for the feedback.

Updated.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, thanks for confirming.

I couldn't remember if we were concerned about shell substitution at one point or not. Certainly shouldn't be a concern for this string.

@jakirkham jakirkham mentioned this pull request Jan 4, 2017
Using " instead of ' is the correct form under Windows and also works on other
platforms
@jakirkham
Copy link
Member

Toggling for CI.

@jakirkham jakirkham closed this Jan 6, 2017
@jakirkham jakirkham reopened this Jan 6, 2017
@jakirkham jakirkham merged commit 6a2be36 into conda-forge:master Jan 6, 2017
@jakirkham
Copy link
Member

Thanks @nicoddemus and @minrk .

@nicoddemus nicoddemus deleted the fix-win-commit-message-hint branch January 6, 2017 11:14
@jakirkham jakirkham added this to the 2.0.0 milestone Jan 9, 2017
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.

None yet

3 participants