Skip to content
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

docs(category/monad,bitraversable): add module docstrings #1260 #1286

Merged
merged 5 commits into from Aug 22, 2019

Conversation

cipher1024
Copy link
Collaborator

TO CONTRIBUTORS:

Make sure you have:

  • reviewed and applied the coding style: coding, naming
  • reviewed and applied the documentation requirements
  • for tactics:
  • make sure definitions and lemmas are put in the right files
  • make sure definitions and lemmas are not redundant

If this PR is related to a discussion on Zulip, please include a link in the discussion.

For reviewers: code review check list

@cipher1024 cipher1024 requested a review from a team as a code owner July 30, 2019 22:02
@cipher1024
Copy link
Collaborator Author

See #1260

Copy link
Member

@robertylewis robertylewis left a comment

Choose a reason for hiding this comment

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

Thanks. This is a good start, but I'm afraid the comments mean very little to me as is. Maybe a good place to start would be documentation for bitraversable.basic? Could you take this opportunity to add declaration doc strings too?

src/category/bitraversable/instances.lean Show resolved Hide resolved
src/category/bitraversable/instances.lean Outdated Show resolved Hide resolved
src/category/bitraversable/lemmas.lean Show resolved Hide resolved
src/category/monad/basic.lean Show resolved Hide resolved
src/category/monad/basic.lean Show resolved Hide resolved
@robertylewis
Copy link
Member

Thanks for the additions, but I'd still like to see more. Right now I don't know what bitraversable is or what I would use these files for. Again, the solution might just be to document bitraversable.basic and relate these comments to there.

@cipher1024
Copy link
Collaborator Author

@robertylewis Before I add a docstring there, do you understand what traversable does? That might be what needs clarification.

@robertylewis
Copy link
Member

Clarifying both would be very helpful!

@cipher1024
Copy link
Collaborator Author

Would the new comment on bitraversable work for traversable? Do you need more information than that?

@robertylewis
Copy link
Member

What I would like from the module docs is to be able to understand the purpose of a file and if/how I might use its contents, without looking through the whole file itself. Some amount of back-referencing is unavoidable, e.g. the p-adic integers doc doesn't repeat everything about the p-adic numbers. But the more of this there is, the harder it is to read, and it shouldn't be necessary to look up external references to get the general idea of a mathlib file.

I'm being picky here because this seems like a programming tool instead of/as well as a mathematical theory. This could be useful for people programming in Lean, but without clear explanations of what it does, they'd never know.

@robertylewis robertylewis added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Aug 22, 2019
@mergify mergify bot merged commit f442a41 into master Aug 22, 2019
@mergify mergify bot deleted the doc-monad-traversable branch August 22, 2019 10:37
cipher1024 added a commit that referenced this pull request Aug 23, 2019
* docs(category/monad,bitraversable): add module docstrings

* more docs

* still more doc

* doc about traversable
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…community#1260  (leanprover-community#1286)

* docs(category/monad,bitraversable): add module docstrings

* more docs

* still more doc

* doc about traversable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants