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

#2317 documentation variable expansion #2966

Merged
merged 4 commits into from
May 27, 2019
Merged

#2317 documentation variable expansion #2966

merged 4 commits into from
May 27, 2019

Conversation

GilbertoCS
Copy link
Contributor

@GilbertoCS GilbertoCS commented Oct 9, 2018

The issue

Documentation should mention variable expansion works only in [[source]] entries #2317

The fix

As suggested I added the use of the operator only in the [[source]] section.
I also simplified the text on the alternatives of usage of variable expansion.

Copy link
Contributor Author

@GilbertoCS GilbertoCS left a comment

Choose a reason for hiding this comment

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

I'm sorry, I accidentally committed the basic.rst, but this is not related to this issue

docs/advanced.rst Outdated Show resolved Hide resolved
@uranusjr
Copy link
Member

uranusjr commented Oct 12, 2018

👍 Could you remove the changes in basic.rst so this merges cleanly? git rebase -i or git cherry-pick could be helpful reforming the commits.

@GilbertoCS
Copy link
Contributor Author

@uranusjr I tried git rebase -i and pushed again, I hope that this time this branch is OK to merge.
thank you for your patience and help.

Copy link
Member

@techalchemy techalchemy left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Overall I would like to see the changes focused on conveying the meaning mentioned in the PR, and the additional changes about the windows variable reverted. Once we get that together, we can go ahead and move forward. Thanks :)

docs/advanced.rst Outdated Show resolved Hide resolved
docs/advanced.rst Outdated Show resolved Hide resolved
docs/advanced.rst Outdated Show resolved Hide resolved
@techalchemy techalchemy added Type: Documentation 📖 This issue relates to documentation of pipenv. DO NOT MERGE Status: Awaiting Update ⏳ This issue requires more information before assistance can be provided. labels Nov 25, 2018
@techalchemy
Copy link
Member

If you don't have the time to finish this up I can go ahead and make the final changes for you, just let me know

@GilbertoCS
Copy link
Contributor Author

If you don't have the time to finish this up I can go ahead and make the final changes for you, just let me know

Hi, so sorry for the delay,
Yes you can make the final changes, thank oy for the time and patience.

@GilbertoCS GilbertoCS closed this Mar 20, 2019
@GilbertoCS GilbertoCS reopened this Mar 20, 2019
@techalchemy techalchemy self-assigned this May 27, 2019
@techalchemy techalchemy added Priority: Low This item is low priority and may not be looked at in the next few release cycles. Status: In Progress This item is in progress. and removed DO NOT MERGE Status: Awaiting Update ⏳ This issue requires more information before assistance can be provided. labels May 27, 2019
Signed-off-by: Dan Ryan <[email protected]>
@techalchemy techalchemy merged commit 8ae44bc into pypa:master May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low This item is low priority and may not be looked at in the next few release cycles. Status: In Progress This item is in progress. Type: Documentation 📖 This issue relates to documentation of pipenv.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation should mention variable expansion works only in [[source]] entries
3 participants