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

doc(data/padics, data/real/cau_seq, algebra): add doc strings, remove unnecessary assumptions #1283

Merged
merged 13 commits into from Jul 31, 2019

Conversation

robertylewis
Copy link
Member

This fixes a few declarations from #1282 (as well as one that didn't show up on that list for some reason), and adds documentation as per #1260 .

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

@robertylewis robertylewis requested a review from a team as a code owner July 30, 2019 09:52
@robertylewis robertylewis changed the title doc(data/padics): add doc strings, remove unnecessary prime assumptions doc(data/padics, data/real/cau_seq, algebra): add doc strings, remove unnecessary prime assumptions Jul 30, 2019
@robertylewis robertylewis changed the title doc(data/padics, data/real/cau_seq, algebra): add doc strings, remove unnecessary prime assumptions doc(data/padics, data/real/cau_seq, algebra): add doc strings, remove unnecessary assumptions Jul 30, 2019
Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

This seems to sit on top of your doc PR for the padics. Is that intentional?

src/algebra/field.lean Outdated Show resolved Hide resolved
@robertylewis
Copy link
Member Author

It's a general "cleanup" PR, I figured I might as well do it all together. (This is it, no more coming.)

@jcommelin
Copy link
Member

Ok, I see. Well, I guess we can merge it as soon as Travis confirms that you didn't clean up too much.

@robertylewis
Copy link
Member Author

Okay, this version definitely builds (and fixes a cascade of unnecessary arguments).

@robertylewis
Copy link
Member Author

Thanks @sgouezel , good catch! Updated.

@sgouezel sgouezel 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 Jul 31, 2019
@mergify mergify bot merged commit 49be50f into master Jul 31, 2019
@mergify mergify bot deleted the extra_args_padic branch July 31, 2019 16:48
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
… unnecessary assumptions (leanprover-community#1283)

* doc(data/padics): add doc strings, remove unnecessary prime assumptions

* fix(data/real/cau_seq): remove unnecessary hypotheses

* fix(algebra/{field, ordered_field}): remove unused assumptions

* doc(data/real/cau_seq): document Cauchy sequences

* fix(algebra/field): remove obsolete lemma

* fix build

* fix build

* more unnecessary arguments

* Update src/data/padics/padic_numbers.lean

* Update src/data/padics/padic_numbers.lean

* remove another unnecessary argument (suggested by @sgouezel)
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

3 participants