Skip to content

bpo-41234: Remove symbol.sym_name #21381

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

Closed

Conversation

nanjekyejoannah
Copy link
Contributor

@nanjekyejoannah nanjekyejoannah commented Jul 7, 2020

@@ -249,3 +249,6 @@ Removed
* Removed ``PyUnicode_AsUnicodeCopy()``. Please use :c:func:`PyUnicode_AsUCS4Copy` or
:c:func:`PyUnicode_AsWideCharString`
(Contributed by Inada Naoki in :issue:`41103`.)

* Removed docs for the deprecated and removed ``symbol.sym_name``.
Copy link
Member

Choose a reason for hiding this comment

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

I think that there should be one entry for the removal of the symbol name and its doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know have a reference on where the entry for the implementation removal is? I did not see any that is why I made this entry.

Copy link
Member

Choose a reason for hiding this comment

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

I did not either. Considering only symbol.h, this entry should say something like
"Removed :module:symbol. (Contributed by Lysandros Nikolaou in :issue: 40939 and Joannah Nanjekye in :issue:41234." (Note: I am guessing at the :module: role. Perhaps is presence is a mistake when the module and doc is removed. Removing the test and docs for a module and other references in the docs are part of removing a module. Finding the latter is non-trivial.)

However, this entry is currently in the C-API section. It belongs in the currently empty Removed section a couple of section after Improved Modules.

The bigger problem is that the removal of symbol is a small part of the removal of the old parser and associated stuff done in 40939, none of which appears in What's New. I believe other modules were removed. So it would be good if you expanded this issue and went through that issue, listed all the removed modules (I don't think that removed tests need to be listed), removed all the associated docs still present, and wrote one module removal entry. The contributed part might only need the addition of "Pablo Galindo and ".

"Removed modules associated with the old parser: (Contributed ...)

A followup would be to do the same at least for public C-API removals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, I think I found this as sort of unfinished business of a former removal of things i.e we have docs for a feature that was removed already which is misleading. I have not bothered to look into the parser removal details though. I have done the required edit in the news entry.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@serverwentdown
Copy link
Contributor

Resolved in PR #21624, can close.

@benjaminp benjaminp closed this Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants