Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

doc improvements at package git epic #231

Closed
31 of 32 tasks
Serabe opened this issue Jan 30, 2017 · 3 comments
Closed
31 of 32 tasks

doc improvements at package git epic #231

Serabe opened this issue Jan 30, 2017 · 3 comments

Comments

@Serabe
Copy link
Contributor

Serabe commented Jan 30, 2017

  • Review package doc (remove references to it being a low-level package, etc.)
  • Highlight general steps to work with repositories. Guide the user a bit through the documentation.
  • Why Commit is treated like a getter while Push, Pull, etc. are treated like commands?
  • Review References function documentation:
    • States that it returns a References while it does not.
    • Does not specify what the commit input is for.
    • State if the ordering is from newer to older or the other way around.
    • Reword sentence "It stops searching a branch for a file upon reaching the commit were the file was created."
    • Fix Caveats section.
  • Document BlameResult
  • In Blame function:
    • Review output documentation. It states that the last commit is returns, but the return type is not an object.Commit.
    • Review input. It states that there are three inputs, but I can only see two in the signature. What is the commit for? What is the path for? Why is the output documented again after the input with a different version of it?
    • Says that Line has a reference to the last commit it was modified on, but the Line struct does not seem to have a commit in it.
    • Each node (line) holds the commit where it was introduced or last modified. To achieve that we use the FORWARD algorithm. The previous phrase action is holding a commit, so I guess we are not using an algorithm to just hold a commit but to find which commit. Rephrase so the pronoun references what is intended to reference.
    • See General questions about docs later in this issue.
  • Line documentation should be synced with that of Blame.
  • XOptions documentation:
    • Typos fixed in PR Fix typos in git docs #230
    • Explain Validate, it seems like a function the user should not care about. If the user should, please state that it is mandatory to be called since it is the one setting the default value.
  • Remote:
    • ELI5 Config.
    • Why do we need to add the remote name to XOptions if the action is being done in a Remote?
  • Repository:
    • Struct doc is messed up.
    • Update NewXRepository to reference go-billy.
    • Either add the default version (if you want to use a custom one you need to use the function NewRepository and build you filesystem.Storage) to all NewXRepository methods or, even better, add this to the struct.
    • NewRepository is outdated respect to its signature.
    • Unify documentation for getters that return an iterator. Specify clearly that the returned type is an iter and specify the type of the objects being iterated over. In case it is needed, specify the order.
    • What is the difference between Objects and Tags? Documentation looks the same.
  • Storer:
    • Reference where the user can find implementations or create their own. Rewords the part of implementations

General questions about docs:

  • Should we avoid adding TODOs to godocs? IMHO, TODOs are not documentation and the user should not care about them. If anything about them needs to be added to the doc, is which things are not supported.
  • Should we avoid stating how things are implemented? IMHO, documentation is not about how something works but why it is there, how the user can take advantage of it, etc. If user needs to know anything about implementation, I would say it is limited to algorithmic complexity.
@mcuadros mcuadros changed the title [DOC] Package git epic. doc improvements at package git epic Jan 30, 2017
@smola smola modified the milestone: v4.0.0-rc10 Feb 21, 2017
@ajnavarro
Copy link
Contributor

ajnavarro commented Feb 28, 2017

@Serabe what do you want to say with this?:

Why Commit is treated like a getter while Push, Pull, etc. are treated like commands?

Commit is a getter because you are getting a commit object. The Commit action is not implemented yet.

Maybe we should change the method name to GetCommit (or whatever) to avoid collisions with the future Commit action implementation.

@ajnavarro
Copy link
Contributor

@Serabe Could you explain me that?:

Why do we need to add the remote name to XOptions if the action is being done in a Remote?

thanks!

ajnavarro added a commit that referenced this issue Mar 3, 2017
@Serabe
Copy link
Contributor Author

Serabe commented Mar 3, 2017

Yay!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants