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

Joss review update #236

Merged
merged 20 commits into from
Sep 14, 2023
Merged

Joss review update #236

merged 20 commits into from
Sep 14, 2023

Conversation

esclapez
Copy link
Collaborator

Hi @baperry2 @nickwimer !

This PR address most of the issues raised by the JOSS reviewers:

  • refactoring of the main README for completeness
  • re-arranging the Exec/RegTests READMEs
  • reducing the number of typo
  • adding a small tutorial on the HotBubble case

I'll get cracking on the change to the paper itself in the JOSS_paper branch.

@esclapez esclapez requested review from nickwimer and baperry2 August 29, 2023 13:52
@baperry2
Copy link
Collaborator

I merged in changes required to get tests to pass since something broke with using wget to get sundials. As a result, there's another environment variable (SUNDIALS_PATH) that can optionally be specified. I'll propagate that change into the readme, tutorials, etc. I also see a small typos, I'll try to get those as well before this gets merged.

Copy link
Collaborator

@baperry2 baperry2 left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for doing this, Lucas. I think this covers pretty much all of the comments, although it may still be good to expand the beginning of the README with more of a statement of need / intended uses.

I fixed some typos and added a bit to the instructions for discussions/issues. Feel free to modify these changes if you'd like.

@esclapez esclapez merged commit 3e9f585 into development Sep 14, 2023
24 checks passed
@esclapez esclapez deleted the joss_review branch September 14, 2023 08:03
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.

2 participants