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

Small README changes #56

Merged
merged 7 commits into from
Nov 23, 2024
Merged

Small README changes #56

merged 7 commits into from
Nov 23, 2024

Conversation

rwegener2
Copy link
Contributor

This PR makes two small suggestions to the README:

  1. Fixes a typo
  2. Adds description of how to run tests

For the tests description of course please re-write, move to another section, or change as you see fit (you should have edit access to this branch). Just wanted to put in a placeholder.

openjournals/joss-reviews#7406

Copy link
Contributor Author

@rwegener2 rwegener2 Nov 22, 2024

Choose a reason for hiding this comment

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

Line 83 is really interesting, but I worry it is a bit dense for someone with less programming background. I would consider adding some higher level text before the denser paragraph, for example maybe
"NdbcApi provides a singular access point for NDBC data. All the methods for finding data, metadata, and locating stations are available using the api object."

Copy link
Owner

Choose a reason for hiding this comment

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

Thank you for this suggestion; I agree and have included this change in the PR you opened.

While I enjoyed building these features when initially designing the API, they are all but irrelevant to end users.

Additionally, I've split off the test instructions into a new CONTRIBUTING.md guide.

@rwegener2
Copy link
Contributor Author

One more comment -- I found it took a moment to understanding "modes". I will suggest a more thorough explanation of modes in the example notebook, but 1-2 short sentences explaining that data variables are grouped into modes here in the README may also be helpful.

@CDJellen CDJellen self-requested a review November 22, 2024 22:47
@CDJellen CDJellen added documentation Improvements or additions to documentation enhancement New feature or request labels Nov 22, 2024
@CDJellen CDJellen self-assigned this Nov 22, 2024
@CDJellen
Copy link
Owner

One more comment -- I found it took a moment to understanding "modes". I will suggest a more thorough explanation of modes in the example notebook, but 1-2 short sentences explaining that data variables are grouped into modes here in the README may also be helpful.

Thank you for pointing this out; I've made some updates to the README to contextualize each of the mode identifiers.

@CDJellen CDJellen merged commit 9745f25 into CDJellen:main Nov 23, 2024
4 checks passed
@rwegener2 rwegener2 deleted the patch-1 branch November 26, 2024 14:39
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 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants