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

Documentation improvements #3

Merged
merged 16 commits into from
Jun 26, 2023
Merged

Documentation improvements #3

merged 16 commits into from
Jun 26, 2023

Conversation

katoss
Copy link
Owner

@katoss katoss commented Jun 23, 2023

Hello,

I adapted the documentation according to your feedback :)

Here is the list of changes:

Statement of need/stating problems and target audience in README: It is succinct but sufficient. However, I think the repo would benefit from a little more explanation for non-experts. This may be as easy as providing a link to an url explaining why card sorting is useful in UX design. (@Robaina )

Added a sentence and a link for non-experts

Installation instructions: for the development version of the package and any non-standard dependencies in README: The instructions are located within the 'CONTRIBUTING.md' file. I would add the link to this file directly in the Contributing section of the README.md file. (@Robaina)

Done

The current name of the package, 'cardsort', is missing as such in the title of the README file. / the title of the README is not exactly the package name (@Robaina , @khynder)

Done, name is now on top in logo

Also, if you like, you could create a simple logo for the package to attract more users (See this repo as an example) (@Robaina )

Made a simple logo and adapted the title to include a little description, inspired by crowsetta

There are no badges in the README. You can read about badges here in case you are not familiar. Also, I can help with this if you need it. (@Robaina) / No badges for now. NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.) (@khynder)

Added badges from the list (CI, tests, docs, repostatus, python versions, pypi version). I have not added a table so far yet, but will keep that in mind in case more badges will be added.

Citation information: No citation info was provided. I suggest publishing the repo in Zenodo and adding the citation and the DOI to the README.md file. Once published, Zenodo provides a badge containing the DOI and which you can display in the badge section of the README file. You could also add a CITATON.cff file to the repo so users can easily get the citation string. (@Robaina) / (@khynder)

So far I have not added citation information. I thought it might be better to wait with publishing on zenodo as a last step when the review is basically done? But I'm not sure how this is usually handled.

'example-data.csv`is located within /docs, you could indicate the relative path in the example to help users find the file within the repo. (@Robaina)

Done

A “)” is missing at the end of the line starting with "distance_matrix", in the “Adapting the dendrogram” section. (@khynder)

Done

Vignette(s) demonstrating major functionality that runs successfully locally. @Batalex what are the "vignettes" ? (@khynder)

As far as I understood, vignettes are examples demonstrating core functionalities. So that should be covered with the examples in the docs and readme. Is that true @Batalex ?

I have not linked the commit references this time, because most of the changes are in the readme and should be easy to spot. If it would be better to have the references, just tell me.

Have a nice evening,
@katoss

@katoss katoss requested review from Batalex and Robaina June 23, 2023 17:19
@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (04014a8) 79.38% compared to head (b5f16ee) 79.38%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #3   +/-   ##
=======================================
  Coverage   79.38%   79.38%           
=======================================
  Files           2        2           
  Lines          97       97           
=======================================
  Hits           77       77           
  Misses         20       20           

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

@katoss katoss added the documentation Improvements or additions to documentation label Jun 23, 2023
@Batalex
Copy link
Collaborator

Batalex commented Jun 26, 2023

As far as I understood, vignettes are examples demonstrating core functionalities. So that should be covered with the examples in the docs and readme. Is that true @Batalex ?

Yes, vignettes are code equivalents of "elevator pitches": a guide showcasing package usage(s) concisely and to the point.

Copy link
Collaborator

@Robaina Robaina left a comment

Choose a reason for hiding this comment

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

LGTM!

@katoss katoss marked this pull request as ready for review June 26, 2023 15:02
@katoss katoss merged commit f7c7675 into main Jun 26, 2023
@katoss katoss deleted the documentation branch June 26, 2023 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants