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

Update docs about variables and aliases #8200

Merged
merged 6 commits into from Jan 30, 2020
Merged

Conversation

@TH3CHARLie
Copy link
Contributor

TH3CHARLie commented Dec 24, 2019

resolves #3494(Since module types are tracked in #3500)

Following #8187 (comment) and #3494, if I understand correctly and the semantics in #3494's example code has been fixed(report error on re-assign Alias = B, the remaining work of #3494 is to update the docs, which is the main focus of this PR.

Newly added docs are in common issues and solutions section, with the content mostly adapted from Ivan's example in #3494. And a note point to the docs is also added.

When reviewing the PR, I think there are two things to tweak:

  • The section of the docs: common issues or alias type ?
  • The wording: is current example and explantion enough for users to understand the difference
@TH3CHARLie

This comment has been minimized.

Copy link
Contributor Author

TH3CHARLie commented Dec 24, 2019

Many tests are broken due to the newly added note, I'd like to first confirm the wording and availability of this PR before pervasively modifying the tests

TH3CHARLie added 3 commits Jan 7, 2020
@TH3CHARLie

This comment has been minimized.

Copy link
Contributor Author

TH3CHARLie commented Jan 27, 2020

I almost forget about this PR.....Now I modify the test suite to apply the new change and pass the tests. But I am worried about that this PR may be too pervasive

@TH3CHARLie TH3CHARLie requested a review from ilevkivskyi Jan 27, 2020
@TH3CHARLie TH3CHARLie requested review from JukkaL and msullivan Jan 29, 2020
Copy link
Collaborator

JukkaL left a comment

Looks good, I just have an idea about the section title.

docs/source/common_issues.rst Outdated Show resolved Hide resolved
@TH3CHARLie TH3CHARLie requested a review from JukkaL Jan 30, 2020
@JukkaL
JukkaL approved these changes Jan 30, 2020
Copy link
Collaborator

JukkaL left a comment

Thanks!

@JukkaL JukkaL merged commit ea3c65c into python:master Jan 30, 2020
4 checks passed
4 checks passed
build (windows-py37-32)
Details
build (windows-py37-64)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@TH3CHARLie TH3CHARLie deleted the TH3CHARLie:fix-3494 branch Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.