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

rewrite the module docstring #985

Merged
merged 2 commits into from
Nov 24, 2021
Merged

rewrite the module docstring #985

merged 2 commits into from
Nov 24, 2021

Conversation

johnnychen94
Copy link
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

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
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
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)

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
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
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
Member Author

Choose a reason for hiding this comment

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

See simonster/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

src/Images.jl Outdated Show resolved Hide resolved
src/Images.jl Outdated Show resolved Hide resolved
src/Images.jl Outdated Show resolved Hide resolved
src/Images.jl Outdated Show resolved Hide resolved
src/Images.jl Outdated Show resolved Hide resolved
Co-authored-by: Tim Holy <[email protected]>
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