Skip to content

Conversation

@MelReyCG
Copy link
Contributor

@MelReyCG MelReyCG commented Nov 26, 2025

GEOS Code Rules

Aims at writing down the coding standards for GEOS, focusing on type safety, error handling, parallelism, and performance. Key principles include:

  • Type System
  • Memory Management
  • Error Handling
  • Parallelism
  • Performance
  • Memory / Safety
  • Architecture
  • Testing

These rules ensure code quality, consistency, and maintainability across the GEOS codebase.


This PR is before #3914

@MelReyCG MelReyCG self-assigned this Nov 28, 2025
@MelReyCG MelReyCG changed the title Docs/rey/code rules docs: Code rules Nov 28, 2025
@MelReyCG MelReyCG changed the title docs: Code rules docs: Adding Code rules page Nov 28, 2025
@MelReyCG MelReyCG marked this pull request as ready for review November 28, 2025 09:12
Copy link
Contributor

@jafranc jafranc left a comment

Choose a reason for hiding this comment

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

Looks great ! Maybe there is the work of 2 docs there (or more 😄 )

Comment on lines 1013 to 1019
Additional Validation Guidelines
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

- Use ``setInputFlag()`` to mark parameters as ``REQUIRED`` or ``OPTIONAL``
- Use ``setApplyDefaultValue()`` to provide sensible defaults
- Use ``setRTTypeName()`` for runtime type validation (e.g., ``rtTypes::CustomTypes::positive``)
- Document valid ranges in ``setDescription()``
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be in the Wrapper section ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It feels strange to me, as the wrapper section is for documentation requirement. My goal here is to precise validation rules. What do you think?

@MelReyCG
Copy link
Contributor Author

MelReyCG commented Dec 3, 2025

Thanks @jafranc for your feedback, I'll take them into account.

@MelReyCG MelReyCG added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Jan 5, 2026
Copy link
Contributor

@jafranc jafranc left a comment

Choose a reason for hiding this comment

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

Great doc !!

**Avoid magic values**:

- **arbitrary values should not be written more than once**, define constants, consider using or extending ``PhysicsConstants.hpp`` / ``Units.hpp``,
- **Prefer to let appear the calculus of constants** rather than writing its value directly without explaination (constexpr has no runtime cost).
Copy link
Contributor

Choose a reason for hiding this comment

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

I would be tempted to include a smooth transition and link to the next page CodeGeosFeatures.rst

@MelReyCG MelReyCG added flag: no rebaseline Does not require rebaseline ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI and removed ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Feb 2, 2026
Wrapper Documentation (User-Oriented)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

All data repository wrappers must be documented with ``setDescription()``.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should encourage the specification of units where this makes sense.

@MelReyCG MelReyCG added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI and removed ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Feb 10, 2026
@MelReyCG MelReyCG added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: no rebaseline Does not require rebaseline flag: ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants