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

Document how the dirty __eq__ is called #41

Merged
merged 4 commits into from
May 13, 2022

Conversation

Marco-Kaulea
Copy link
Contributor

Closes #38

@codecov
Copy link

codecov bot commented May 13, 2022

Codecov Report

Merging #41 (b9a63b4) into main (2d3c0e5) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #41   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          660       660           
  Branches       167       167           
=========================================
  Hits           660       660           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d3c0e5...b9a63b4. Read the comment docs.

Copy link
Owner

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

Thanks so much for investigating this.

Arguably this belongs more in the docs than in the readme, but I'm happy for it to be included in both. Please could you copy or move this into docs/index.md or a new file in docs.

Please could you also wrap this content to match the style of the rest of the readme and docs.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@Marco-Kaulea
Copy link
Contributor Author

I wasn't sure where to put it initially, so the bottom of the README seemed as good a place as any. But on second though it's probably not important enough.

I hope the line wrapping is OK this way, that was my editor default.

Copy link
Owner

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

Some more typos :-)

docs/internals.md Outdated Show resolved Hide resolved
docs/internals.md Outdated Show resolved Hide resolved
docs/internals.md Outdated Show resolved Hide resolved
@samuelcolvin
Copy link
Owner

You need to add internals.md to mkdocs.yml, you can see in the preview that the file isn't rendered. You might also want to change the heading levels to 1 and 2.

@samuelcolvin
Copy link
Owner

or you can add this content to the bottom of index.md, whichever you prefer.

@Marco-Kaulea
Copy link
Contributor Author

No idea where I can find the current preview. I hope this works now.
Either way, I'm off of work now and might not check back in for a while.

@samuelcolvin
Copy link
Owner

See "smokeshow / docs — Docs Preview" in the "Checks".

That looks great, thank you so much.

@samuelcolvin samuelcolvin merged commit d4e5a77 into samuelcolvin:main May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include documentation on how this works
2 participants