jeff-cohere commented 3 years ago

This is all negotiable. It's just a starting point, not intended to offend anyone. :-) Please add whoever you think might care about this stuff to the list of reviewers.

Closes #118

codecov-io commented 3 years ago

Codecov Report

Merging #119 (4f75bd5) into master (49036a8) will increase coverage by 0.83%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
+ Coverage   85.24%   86.07%   +0.83%     
==========================================
  Files          49       52       +3     
  Lines       10540    12469    +1929     
==========================================
+ Hits         8985    10733    +1748     
- Misses       1555     1736     +181

Impacted Files	Coverage Δ
src/tdywy.c	`85.12% <0.00%> (-0.39%)`	:arrow_down:
src/mpfao/3D/tdympfao3D_ts.c	`98.53% <0.00%> (-0.13%)`	:arrow_down:
src/mpfao/2D/tdympfao2D.c	`100.00% <0.00%> (ø)`
src/mpfao/3D/tdympfao3D_steady.c	`100.00% <0.00%> (ø)`
src/mpfao/3D/tdympfao3D_ts_dae.c	`0.00% <0.00%> (ø)`
src/mpfao/3D/tdympfao3D_ts_transientvar.c	`0.00% <0.00%> (ø)`
src/tdymaterialproperties.c	`67.50% <0.00%> (ø)`
src/tdycharacteristiccurves.c	`68.21% <0.00%> (ø)`
src/tdyconditions.c	`54.05% <0.00%> (ø)`
src/mpfao/3D/tdympfao3D_core.c	`97.41% <0.00%> (+0.05%)`	:arrow_up:
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 49036a8...4f75bd5. Read the comment docs.

bishtgautam commented 3 years ago

It is easier to read the changes here.

bishtgautam commented 3 years ago

@jedbrown Is there a libCEED style guide?

jedbrown commented 3 years ago

libCEED uses astyle and clang-tidy (also run via CI) for style/coding standards. We don't have a formal document on the object model, but the number of types is relatively small.

jeff-cohere commented 3 years ago

Thanks for the feedback, everyone. I've added a section on polymorphism, updated the section on constructors, and floating point number comparisons (functions described to be implemented). If no one objects, I'll merge this PR today. We can always revisit anything in this guide.

nocollier commented 3 years ago

I am very late to this game, but I had a few rough comments:

I do not have a personal sense of what style should be adhered to. Nor have I ever read a style guide. I mostly just try to stick to the style I see around me.
I like this guide because it explains why some things are the way they are. I learned a lot about coding just from repeating what I saw others do, but the underlying reasons often make sense. They just aren't clear from the outset. A guide like this I think has the possibility to bring others along more quickly. Keeping it relatively brief is key.
I am clearly in the PETSc camp. I admire their work and feel they have (mostly?) good reasons for what they do. I am happy to blindly adopt whatever they do.

On Fri, Nov 13, 2020 at 5:29 PM Jeffrey N. Johnson notifications@github.com wrote:

Merged #119 https://github.com/TDycores-Project/TDycore/pull/119 into master.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/TDycores-Project/TDycore/pull/119#event-3995608435, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKFCB4ORARQ4SPUYYMBIZLSPWXLFANCNFSM4TRFCLQA .

jeff-cohere commented 3 years ago

Thanks, @nocollier . Are there any changes you would suggest to what's written so far, or anything you feel could be better explained?

TDycores-Project / TDycore

Rough draft of a style guide. Let the holy wars begin! #119

Codecov Report