[webkit-reviews] review granted: [Bug 221986] [webkitscmpy] Document package usage : [Attachment 420541] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 16 15:04:32 PST 2021
Stephanie Lewis <slewis at apple.com> has granted Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 221986: [webkitscmpy] Document package usage
https://bugs.webkit.org/show_bug.cgi?id=221986
Attachment 420541: Patch
https://bugs.webkit.org/attachment.cgi?id=420541&action=review
--- Comment #3 from Stephanie Lewis <slewis at apple.com> ---
Comment on attachment 420541
--> https://bugs.webkit.org/attachment.cgi?id=420541
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=420541&action=review
> Tools/Scripts/libraries/webkitscmpy/README.md:7
> +webkitcorepy along with fasteners, monotonic, whichcraft, and xmltodict.
nit: Think this would read better as bulleted list than as a sentence
> Tools/Scripts/libraries/webkitscmpy/README.md:11
> +The `git-webkit` command exposes much of the utility of the `webkitscmpy`
library to the command line. Most notably:
nit: the utility of webktscmpy seems circular. We haven't defined what the
utility is yet. Maybe replace with what the utility of webkitscmpy is
> Tools/Scripts/libraries/webkitscmpy/README.md:15
> +`git-webkit checout <ref>`: Move the current local repository to the
provided git ref, Subversion revision or identifier.
typo checkout
> Tools/Scripts/libraries/webkitscmpy/README.md:17
> +`git-webkit canoniclize`: Standardize commit authorship and put identifiers
into the commit message.
typo canonicalize. I think that's an awkward name for a command
> Tools/Scripts/libraries/webkitscmpy/README.md:21
> +The `webkitscmpy` library provides a repository abstraction for both local
and remote repositories. Those abstractions
nit: Instead of Those abstractions can be instantiated in code like so
how about: To instantiate a repository object.
> Tools/Scripts/libraries/webkitscmpy/README.md:28
> +remote.Scm.from_url('https://svn.webkit.org/repository/webkit')
nit: Would this make more sense if it was assigned to a variable?
> Tools/Scripts/libraries/webkitscmpy/README.md:33
> +be missing from certain implementations. For example, remote repositories do
not have a `checkout` command available.
nit how about While the abstraction layer is consistent for all implementations
not all implementation support every feature. For example, ...
> Tools/Scripts/libraries/webkitscmpy/README.md:35
> +Each repository abstraction keeps a record contributors, which can be primed
and passed into the repository object:
nit: record of contributors? list of contributor records?
More information about the webkit-reviews
mailing list