Skip to content

rewrite the module docstring#985

Merged
johnnychen94 merged 2 commits intomasterfrom
jc/module_str
Nov 24, 2021
Merged

rewrite the module docstring#985
johnnychen94 merged 2 commits intomasterfrom
jc/module_str

Conversation

@johnnychen94
Copy link
Copy Markdown
Member

I deliberately separate this into a new PR so we don't get lost in the lengthy discussions in #971

closes #982

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 4, 2021

Codecov Report

Merging #985 (ee81136) into jc/houseclean (972d810) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           jc/houseclean     #985   +/-   ##
==============================================
  Coverage          87.72%   87.72%           
==============================================
  Files                  8        8           
  Lines                823      823           
==============================================
  Hits                 722      722           
  Misses               101      101           
Impacted Files Coverage Δ
src/Images.jl 100.00% <ø> (ø)

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 972d810...ee81136. Read the comment docs.

@timholy
Copy link
Copy Markdown
Member

timholy commented Nov 5, 2021

If we're removing the API summary, perhaps we should make sure that all listed @reexported modules have their own module docstrings. Otherwise there is some loss to this. But I think we could merge this and then fix those.

@johnnychen94
Copy link
Copy Markdown
Member Author

johnnychen94 commented Nov 5, 2021

But I think we could merge this and then fix those.

This seems to be a 1.0 milestone. I plan to have an issue to track all the exported symbols and add necessary docstrings and demo page before 1.0 release. (#986)

Comment thread src/Images.jl Outdated
Comment on lines +108 to +109
The purpose of this package is to have an out-of-box experiences for most of the stable
functionalities. This means when you do `using Images`, you load a lot of packages that
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
The purpose of this package is to have an out-of-box experiences for most of the stable
functionalities. This means when you do `using Images`, you load a lot of packages that
Images provides an out-of-box toolkit for image processing.
This means when you do `using Images`, you load a lot of packages that

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

for most of the stable functionalities.

I do have some concerns with this. I'll write up a new issue to explain it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See JuliaLang/Reexport.jl#37. This is why I think we can't expect Images to provide up-to-date functionalities if we want to follow the semver strictly. We have abused it a lot but I think we need to be more cautious on this after v1.0

Comment thread src/Images.jl Outdated
Comment thread src/Images.jl Outdated
Comment thread src/Images.jl Outdated
Comment thread src/Images.jl Outdated
Comment thread src/Images.jl Outdated
Co-authored-by: Tim Holy <tim.holy@gmail.com>
Base automatically changed from jc/houseclean to master November 24, 2021 15:53
@johnnychen94 johnnychen94 merged commit 3fc7ed3 into master Nov 24, 2021
@johnnychen94 johnnychen94 deleted the jc/module_str branch November 24, 2021 15:55
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.

rewrite the Images docstring

2 participants