forked from gitpython-developers/GitPython
-
Notifications
You must be signed in to change notification settings - Fork 0
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
(Test PR) Investigate CodeQL output (from copyedit
branch)
#4
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This further improves the text previously introduced in gitpython-developers#1829 and improved in gitpython-developers#1844.
The fragment of the refresh failure message (which is often written to a terminal) about the effect of setting GIT_PYTHON_REFRESH to control refesh failure reporting had previously been formatted with a maximum line length of 79 columns--in the message itself, not the code for the message--so that it would fit in a traditional 80 column wide terminal. This remains one of the popular widths to set for terminal windows in a GUI, so it seems worthwhile to preserve. In 3a6e3ef (gitpython-developers#1815), I had inadvertently made the line one character too long for that; at 80 columns, it would cause the newline to be written at the start of the next line, creating an unsightly extra line break. This is pretty minor, but what seems to me like an equally good alternative wording avoids it, so this commit shortens the wording.
Mainly for formatting.
The git.db.partial_to_complete_sha_hex docstring refers to "AmbiguousObjects", suggesting the existence of an AmbiguousObject type (or an AmbiguousObjects type). Since there appears to be no such type in GitPython (or in gitdb), this seems to have been written in reference to the condition expressed by the AmbiguousObjectName exception, which the note says is not currently able to be raised (such that BadObject is raised instead) in situations where it would apply. Since the connection to that exeception is already clear from context, this commit changes the wording to "ambiguous objects" to avoid being misread as a reference to a Python class of ambiguous objects.
This converts it from a specially formatted comment to a docstring (gitpython-developers#1734), rewords for clarity, and removes the mention of unicode, which appears to have been referring to the data type. (GitPython no longer supports Python 2, and unicode is not a separate type from str in Python 3, where instead str and bytes are different types.)
For slightly easier reading.
- Replace the goo.gl web shortlink with a Sphinx reference that displays and also (in built documentation) becomes a hyperlink to the documentation on the method being referred to. - Refer to a related class (which is in another module) as being in the module where it is defined, rather than in the top-level git module (where it also can be accessed because it is imported there, which is reasonable to do, but less clear documentation). - Tweak punctuation so the effect of passing None is a bit clearer.
Except for: - git.cmd, where docstrings were revised in e08066c. - git.types, where docstring changes may best be made together with changes to how imports are organized and documented, which seems not to be in the same scope as the changes in this commit. This change, as well as those in e08066c, are largely along the lines of gitpython-developers#1725, with most revisions here being to docstrings and a few being to comments. The major differences between the kinds of docstring changes here and those ind gitpython-developers#1725 are that the changes here push somewhat harder for consistency and apply some kinds of changes I was reluctant to apply widely in gitpython-developers#1725: - Wrap all docstrings and comments to 88 columns, except for parts that are decisively clearer when not wrapped. Note that semi- paragraph changes represented as single newlines are still kept where meaningful, which is one reason this is not always the same effect as automatic wrapping would produce. - Avoid code formatting (double backticks) for headings that precede sections and code blocks. This was done enough that it seems to have been intentional, but it doesn't really have the right semantics, and the documentation is currently rendering in such a way (including on readthedocs.org) where removing that formatting seems clearly better. - References (single backticks with a role prefix) and code spans (double backticks) everywhere applicable, even in the first lines of docstrings. - Single-backticks around parameter names, with no role prefix. These were mostly either formatted that way or emphasized (with asterisks). This is one of the rare cases that I have used single backticks without a role prefix, which ordinarily should be avoided, but to get a role for references to a function's parameters within that function, a plugin would be needed. In the rare case that one function's docstring refers to another function's parameters by names those are double-backticked as code spans (and where applicable the name of the referred-to function is single-backticked with the :func: or :meth: role). - All sections, such as :param blah:, :note:, and :return:, now have a newline before any text in them. This was already often but far from always done, and the style was overall inconsistent. Of consistent approaches that are clear and easy to write, this is the simplest. It also seems to substantially improve readability, when taken together with... - Sections are always separated by a blank line, even if they are very short. - Essentially unlimited use of `~a.b.c`, where applicable, to refer and link to the documentation for a.b.c while showing the text "a" and revealing "a.b.c" on hover. I had previously somewhat limited my use of this tilde notation in case readers of the source code itself (where it is not rendered) weren't familiar with it, but at the cost of less consistency in when an entity was referred to. There remain a couple places in git.util where I do not do this because the explicit form `a <a.b.c>`, which is equivalent, lined things up better and was thus easier to read. Those are the major differences between the approach taken here and in gitpython-developers#1725, but not necessarily most of the changes done here (many of which are the same kinds of revisions as done there). Note that this commit only modifies some git/*.py files, and there are more git/**/*.py files that remain to be revised accordingly.
This makes it easier to read along with, and compare to, the other overloads.
This didn't how a CodeQL alert for the commit where it was shown in gitpython-developers#1850. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a fork-internal PR for investigating CodeQL output in:
This is to gitpython-developers#1850 as #1 is to gitpython-developers#1813.