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: README update #1195

Merged
merged 24 commits into from
Feb 14, 2023
Merged

DOC: README update #1195

merged 24 commits into from
Feb 14, 2023

Conversation

ZviBaratz
Copy link
Contributor

@ZviBaratz ZviBaratz commented Feb 8, 2023

Organized a bit, removed some older references, added a bunch of badges.
If and when approved, I will copy the contents to info.py.

PREVIEW

@codecov
Copy link

codecov bot commented Feb 8, 2023

Codecov Report

Base: 92.16% // Head: 92.16% // No change to project coverage 👍

Coverage data is based on head (6d1fd30) compared to base (fc49243).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1195   +/-   ##
=======================================
  Coverage   92.16%   92.16%           
=======================================
  Files          97       97           
  Lines       12332    12332           
  Branches     2534     2534           
=======================================
  Hits        11366    11366           
  Misses        645      645           
  Partials      321      321           
Impacted Files Coverage Δ
nibabel/info.py 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Thanks, Zvi! I made a batch of suggestions, but this is overall a tremendous improvement.

As always, feel free to argue back if I have been insufficiently persuasive...

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
ZviBaratz and others added 12 commits February 12, 2023 20:01
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
@ZviBaratz
Copy link
Contributor Author

I'm glad you approve, thank you for all the fixes and suggestions.
Looking at it now, I feel like the "Packaging" section should be removed. Will suggest a fix tomorrow.

Tried merging with other sections and using line breaks for some
inner-section separation.
Line breaks did not work as expected. Split "Code" section to "Code"
and "Tests", and "Distribution" section to "PyPI" and "Linux".
Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

LGTM. Two small comments.

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@ZviBaratz
Copy link
Contributor Author

@effigies re-requested one last review to make sure copying to long_description was done correctly.

Also, @matthew-brett, is this OK with you?

@effigies
Copy link
Member

Some of this doesn't look great in the docs. I'll push a suggestion.

image

* Create top-level header in index.rst
* Remove duplicate definition of MIT License URL
@effigies
Copy link
Member

Updated:

image

@ZviBaratz
Copy link
Contributor Author

Thank you. Not sure whether we should wait any longer before merging this or not 🤷🏼‍♂️

@effigies
Copy link
Member

We're good to go if you're happy with my changes.

@ZviBaratz ZviBaratz merged commit 3f1c6f9 into nipy:master Feb 14, 2023
@ZviBaratz ZviBaratz deleted the doc/readme branch February 14, 2023 13:41
@effigies effigies added this to the 5.1.0 milestone Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants