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

Developing the developer guide #281

Merged
merged 42 commits into from
Jul 19, 2024
Merged

Developing the developer guide #281

merged 42 commits into from
Jul 19, 2024

Conversation

JuliaKukulies
Copy link
Member

@JuliaKukulies JuliaKukulies commented May 12, 2023

This is a draft pull request to collaboratively work on the developer guide.

I have started to suggest some subsections and points to include based on the discussion in #265

Here we can look at how our progress looks like on read the docs: https://tobac--281.org.readthedocs.build/en/281/

@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.71%. Comparing base (2fe56c0) to head (cf3c6bd).
Report is 46 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
+ Coverage   60.44%   60.71%   +0.26%     
==========================================
  Files          23       23              
  Lines        3522     3541      +19     
==========================================
+ Hits         2129     2150      +21     
+ Misses       1393     1391       -2     
Flag Coverage Δ
unittests 60.71% <100.00%> (+0.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JuliaKukulies JuliaKukulies added the documentation Updates and improvements to documentation and examples label May 12, 2023
@JuliaKukulies JuliaKukulies added this to the Version 1.6 milestone May 12, 2023
@fsenf fsenf mentioned this pull request Jul 7, 2023
11 tasks
@fsenf
Copy link
Member

fsenf commented Jul 7, 2023

@JuliaKukulies : I now added the sphinx-based rendering docu to this PR. I used pandoc to convert markdown to rst, but I am not sure with all code parts have been converted in the correct way. Could you have a look?

@JuliaKukulies
Copy link
Member Author

JuliaKukulies commented Jul 8, 2023

Perfect @fsenf ! I added your file to index.rst but for some reason the page does not show up. At a first glance the conversion looks good, but I will check in more detail.

@JuliaKukulies JuliaKukulies mentioned this pull request Jul 12, 2023
11 tasks
@JuliaKukulies JuliaKukulies changed the base branch from RC_v1.5.0 to RC_v1.5.x July 14, 2023 20:14
- mainly minor spelling and formatting corrections
- added exclude pattern to conf.py to limit rendering to the actual content
- changed toctree: excl. sphinx-based doc -> it is now linked in contributing.rst
@fsenf
Copy link
Member

fsenf commented Sep 1, 2023

I made some updates to the dev guide which will come with the next direct commit to the branch.

All looks good beside that these topics still need to be described in detail in doc/contributing.rst

  • Github actions / workflow
  • unit testing -> How to add a unit test
  • jupyter notebook examples

It would be great to get support from @freemansw1 and @w-k-jones for describing the first two items. I could care for the last item.

Should make separate rst file for each topic to make the content easier to diggest?

@JuliaKukulies
Copy link
Member Author

JuliaKukulies commented Sep 1, 2023

Great, thanks for your updates @fsenf ! I think it is a good idea to have separate .rst files for these items as they are a bit more comprehensive. The structure of the dev guide is also just a first draft suggestion, so feel free to change it where necessary!

What should we do with the CONTRIBUTING.md? Do you want to keep it or should we just make sure that all the information is somewhere in the dev guide? I personally find it better to have all on one place and remove CONTRIBUTING.md

@fsenf
Copy link
Member

fsenf commented Sep 4, 2023

My suggestion would be:

  • Keep CONTRIBUTING.md
  • but reduce content to nice welcome words and a link list to the actual content in the dev guide

@JuliaKukulies
Copy link
Member Author

My suggestion would be:

  • Keep CONTRIBUTING.md
  • but reduce content to nice welcome words and a link list to the actual content in the dev guide

I like this suggestion- let's do that!

- reformatted links
- spell corrections
- removed docstring from CONTRIBUTING file
Copy link
Member

@fsenf fsenf left a comment

Choose a reason for hiding this comment

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

This dev doc is in a very good shape! Thank you a lot for your efforts! I just have a few minor comments and incorporated some spelling and formatting corrections.

CONTRIBUTING.md Outdated Show resolved Hide resolved
doc/contributing.rst Outdated Show resolved Hide resolved
doc/contributing.rst Outdated Show resolved Hide resolved
@JuliaKukulies
Copy link
Member Author

Thanks for your comments, @fsenf ! I think I addressed them all :)

Copy link
Member

@fsenf fsenf left a comment

Choose a reason for hiding this comment

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

Once again, great work!

@fsenf
Copy link
Member

fsenf commented May 6, 2024

@freemansw1 : Hi Sean, this PR is ready for a 2nd review. Do you think you can make it until the next dev meeting? Thanks, Fabian.

@fsenf fsenf modified the milestones: Version 1.6, Version 1.5.4 May 17, 2024
@fsenf
Copy link
Member

fsenf commented May 28, 2024

Who agreed to be the 2nd reviewer here? Was it you @freemansw1 ?

@freemansw1
Copy link
Member

Thanks, @JuliaKukulies and @fsenf .

We have one minor merge conflict to fix, but I would also be happy for this to be merged directly into main as it only touches documentation, per our policies around that kind of thing.

@JuliaKukulies
Copy link
Member Author

Not super sure why the zenodo json check fails, since the formatting and syntax seemed to be OK after i updated my affiliation?

@JuliaKukulies JuliaKukulies changed the base branch from RC_v1.5.x to main June 28, 2024 19:25
@JuliaKukulies
Copy link
Member Author

All right, I fixed the json formatting issue, so I am merging directly into main as we have discussed before. Thanks @fsenf and @freemansw1 and lets make sure to continue to work and update the developer guide in the future.

@JuliaKukulies JuliaKukulies merged commit 201c601 into main Jul 19, 2024
38 checks passed
@freemansw1
Copy link
Member

All right, I fixed the json formatting issue, so I am merging directly into main as we have discussed before. Thanks @fsenf and @freemansw1 and lets make sure to continue to work and update the developer guide in the future.

See the JSON formatting CI was making sure that you complied with all new marketing requirements :) (I still don't know what the issue was, but glad it resolved).

@JuliaKukulies
Copy link
Member Author

JuliaKukulies commented Jul 22, 2024

All right, I fixed the json formatting issue, so I am merging directly into main as we have discussed before. Thanks @fsenf and @freemansw1 and lets make sure to continue to work and update the developer guide in the future.

See the JSON formatting CI was making sure that you complied with all new marketing requirements :) (I still don't know what the issue was, but glad it resolved).

Haha goood :)

Also not sure what the issue was, I just copied the json file from another branch and it worked

@freemansw1 freemansw1 mentioned this pull request Jul 24, 2024
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Updates and improvements to documentation and examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants