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

extend documentation: add_new_kernel #125

Merged
merged 6 commits into from
Dec 6, 2022
Merged

Conversation

thawn
Copy link
Collaborator

@thawn thawn commented Dec 1, 2022

This is a first draft extending the documentation for adding a new kernel. I added the sections Add a new method to the Clesperanto class gateway and Add a valid test case.

@thawn thawn requested a review from StRigaud December 1, 2022 21:11
@StRigaud StRigaud added the documentation documentation related topic label Dec 2, 2022
@StRigaud
Copy link
Member

StRigaud commented Dec 2, 2022

Thanks for the help!

Thinking about splitting the markdown file into smaller ones, as follow:

  • how to contribute
  • create a new class
    • header
    • source
    • gateway
  • create a test
    • source
    • cmakelist

Large and long text file make people don't want to read.

@thawn
Copy link
Collaborator Author

thawn commented Dec 2, 2022

@StRigaud that sounds like an excellent idea. How about adding "How to contribute" as a new section to the guidelines document? It seems to fit well there (and that document is not very long yet).

@thawn
Copy link
Collaborator Author

thawn commented Dec 2, 2022

@StRigaud How about splitting the document into two like I just committed? (assuming we add a new how to contribute section to the guidelines)

@codecov-commenter
Copy link

codecov-commenter commented Dec 2, 2022

Codecov Report

Base: 77.88% // Head: 78.01% // Increases project coverage by +0.13% 🎉

Coverage data is based on head (0224382) compared to base (51a3300).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #125      +/-   ##
==========================================
+ Coverage   77.88%   78.01%   +0.13%     
==========================================
  Files         164      167       +3     
  Lines        8043     8097      +54     
==========================================
+ Hits         6264     6317      +53     
- Misses       1779     1780       +1     
Impacted Files Coverage Δ
clic/include/core/clesperanto.hpp 91.66% <0.00%> (ø)
tests/convolve_test.cpp 95.83% <0.00%> (ø)
clic/src/tier1/cleConvolveKernel.cpp 100.00% <0.00%> (ø)
clic/include/tier1/cleConvolveKernel.hpp 100.00% <0.00%> (ø)
clic/src/core/clesperanto.cpp 89.36% <0.00%> (+0.06%) ⬆️

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.

@StRigaud
Copy link
Member

StRigaud commented Dec 2, 2022

Yes this is good. No need to over-split neither 😉

How about adding "How to contribute" as a new section to the guidelines document? It seems to fit well there (and that document is not very long yet).

Yes! the guideline a here as a summary top file from which we can link to sub-documentation.

@StRigaud
Copy link
Member

StRigaud commented Dec 2, 2022

Base: 77.88% // Head: 78.01% // Increases project coverage by +0.13%

Since when writing doc increase coverage?

@thawn
Copy link
Collaborator Author

thawn commented Dec 2, 2022

I was also surprised by this.

I guess, writing documentation decreases the percentage of code - hence the test coverage of the entire project (code + documentation) is increased - still funny though ;-)

@StRigaud
Copy link
Member

StRigaud commented Dec 2, 2022

a vicious incentive for pushing dev into writing documentation ;)

@StRigaud
Copy link
Member

StRigaud commented Dec 6, 2022

Rearrange of the doc, plus added a few more on coding guideline and clang-format.

Don't know if we need more for now.

@thawn
Copy link
Collaborator Author

thawn commented Dec 6, 2022

I just read your changes - looks great to me!
ready to merge from my side 👍

@thawn thawn marked this pull request as ready for review December 6, 2022 12:56
@StRigaud StRigaud merged commit 02ac460 into master Dec 6, 2022
@StRigaud StRigaud deleted the extend_doc_add_new_kernel branch December 6, 2022 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation documentation related topic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants