You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by "ocket8888 (via GitHub)" <gi...@apache.org> on 2023/09/12 18:12:03 UTC

[GitHub] [trafficcontrol] ocket8888 opened a new pull request, #7800: Add the ability to markup things as being removed in the documentation

ocket8888 opened a new pull request, #7800:
URL: https://github.com/apache/trafficcontrol/pull/7800

   This adds a `.. versionremoved::` directive to our documentation so we can make note of the version from which a thing will be unavailable. As an example usage, it's been added to the v4 page for the now-removed `deliveryservices_required_capabilities` API endpoint.
   
   It also makes some updates to the ATC Sphinx extension to keep it more in line with our Python standards:
   
   - Move imports to the top of the file
   - Ensure type hints everywhere
   - Add docstrings wherever they're missing
   
   <hr/>
   
   ## Which Traffic Control components are affected by this PR?
   - Documentation
   
   ## What is the best way to verify this PR?
   Make sure the docs build without errors or warnings.
   
   ## PR submission checklist
   - [x] This PR doesn't need tests
   - [x] This PR is documentation
   - [x] This PR doesn't need a CHANGELOG.md entry
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] zrhoffman merged pull request #7800: Add the ability to markup things as being removed in the documentation

Posted by "zrhoffman (via GitHub)" <gi...@apache.org>.
zrhoffman merged PR #7800:
URL: https://github.com/apache/trafficcontrol/pull/7800


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7800: Add the ability to markup things as being removed in the documentation

Posted by "zrhoffman (via GitHub)" <gi...@apache.org>.
zrhoffman commented on code in PR #7800:
URL: https://github.com/apache/trafficcontrol/pull/7800#discussion_r1337938258


##########
docs/source/_ext/atc.py:
##########
@@ -11,158 +15,260 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
+import os
+import re
+from typing import Any, Final, Optional
 
-REPO_URI = "https://github.com/apache/trafficcontrol/"
-
-# -- Implementation detail directive -----------------------------------------
-from docutils import nodes
+from docutils import nodes, utils
+from sphinx.application import Sphinx
 from sphinx.util.docutils import SphinxDirective
-from sphinx.locale import translators, _
+from sphinx.locale import _
+from sphinx.domains import changeset
+
+
+REPO_URI: Final = "https://github.com/apache/trafficcontrol/"
 
 class impl(nodes.Admonition, nodes.Element):
-	pass
+	"""
+	This class is just used to provide translatability for the documentation.
+	Which is currently unimplemented, but the translators will choke on
+	directives that don't have visit and depart methods defined, so if that's
+	ever added in the future the sort of "incomplete" definitions of directives
+	here would be a problem.
+	"""
 
-def visit_impl_node(self, node):
+def visit_impl_node(self: nodes.Admonition, node: nodes.Node):
+	"""
+	Implementation of Node entry for custom admonitions for translation
+	purposes.
+	"""
 	self.visit_admonition(node)
 
-def depart_impl_node(self, node):
+def depart_impl_node(self: nodes.Admonition, node: nodes.Node):
+	"""
+	Implementation of Node entry for custom admonitions for translation
+	purposes.
+	"""
 	self.depart_admonition(node)
 
+# -- Implementation detail directive -----------------------------------------
 class ImplementationDetail(SphinxDirective):
+	"""
+	An admonition that describes something that is possibly interesting or
+	informative context for our particular implementation of something.
+
+	Example:
 
+		.. impl-detail::
+			This is because the underlying number is 32 bits wide.
+	"""
 	has_content = True
 	required_arguments = 0
 	optional_arguments = 1
 	final_argument_whitespace = True
 
-	label_text = 'Implementation Detail'
+	label_text = "Implementation Detail"
 
-	def run(self):
-		impl_node = impl('\n'.join(self.content))
+	def run(self) -> list[nodes.Node]:

Review Comment:
   Documentation fails to build
   
   ```python
   File "/github/workspace/docs/source/_ext/atc.py", line 72, in ImplementationDetail
       def run(self) -> list[nodes.Node]:
   TypeError: 'type' object is not subscriptable
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7800: Add the ability to markup things as being removed in the documentation

Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7800:
URL: https://github.com/apache/trafficcontrol/pull/7800#discussion_r1338750909


##########
docs/source/_ext/atc.py:
##########
@@ -11,158 +15,260 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
+import os
+import re
+from typing import Any, Final, Optional
 
-REPO_URI = "https://github.com/apache/trafficcontrol/"
-
-# -- Implementation detail directive -----------------------------------------
-from docutils import nodes
+from docutils import nodes, utils
+from sphinx.application import Sphinx
 from sphinx.util.docutils import SphinxDirective
-from sphinx.locale import translators, _
+from sphinx.locale import _
+from sphinx.domains import changeset
+
+
+REPO_URI: Final = "https://github.com/apache/trafficcontrol/"
 
 class impl(nodes.Admonition, nodes.Element):
-	pass
+	"""
+	This class is just used to provide translatability for the documentation.
+	Which is currently unimplemented, but the translators will choke on
+	directives that don't have visit and depart methods defined, so if that's
+	ever added in the future the sort of "incomplete" definitions of directives
+	here would be a problem.
+	"""
 
-def visit_impl_node(self, node):
+def visit_impl_node(self: nodes.Admonition, node: nodes.Node):
+	"""
+	Implementation of Node entry for custom admonitions for translation
+	purposes.
+	"""
 	self.visit_admonition(node)
 
-def depart_impl_node(self, node):
+def depart_impl_node(self: nodes.Admonition, node: nodes.Node):
+	"""
+	Implementation of Node entry for custom admonitions for translation
+	purposes.
+	"""
 	self.depart_admonition(node)
 
+# -- Implementation detail directive -----------------------------------------
 class ImplementationDetail(SphinxDirective):
+	"""
+	An admonition that describes something that is possibly interesting or
+	informative context for our particular implementation of something.
+
+	Example:
 
+		.. impl-detail::
+			This is because the underlying number is 32 bits wide.
+	"""
 	has_content = True
 	required_arguments = 0
 	optional_arguments = 1
 	final_argument_whitespace = True
 
-	label_text = 'Implementation Detail'
+	label_text = "Implementation Detail"
 
-	def run(self):
-		impl_node = impl('\n'.join(self.content))
+	def run(self) -> list[nodes.Node]:

Review Comment:
   Yeah, I was trying to find a way to update the Python version used by the action, but that doesn't seem viable. So I may have to lean on now-redundant meta types from `typing`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org