Skip to content

Adding build developer documentation #222

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Adding build developer documentation #222

wants to merge 4 commits into from
+837 −1

Conversation

@pmady
Copy link

@pmady pmady commented Nov 30, 2025 •
edited
Loading

Description

Tests

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable. (Check if there is no
    need to update the documentation, for example if this is a bug fix that
    doesn't change the API.)
  • I have ensured that the change is tested somewhere in the testsuite
    (adding new test cases if necessary).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format before submitting, I definitely will look at the CI
    test that runs clang-format and fix anything that it highlights as being
    nonconforming.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 30, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@pmady pmady mentioned this pull request Nov 30, 2025
Open
@codecov-commenter
Copy link

codecov-commenter commented Nov 30, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.49%. Comparing base (0b3db70) to head (8fa95ac).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #222   +/-   ##
=======================================
  Coverage   81.49%   81.49%           
=======================================
  Files          11       11           
  Lines        2151     2151           
  Branches      327      327           
=======================================
  Hits         1753     1753           
  Misses        398      398

Continue to review full report in Codecov by Sentry.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pmady
Copy link
Author

pmady commented Dec 2, 2025

Any reviewers for this PR. As it is my first time contribution curious if I made an mistakes/ open for feedback. Thanks

@antond-weta
Copy link
Contributor

antond-weta commented Dec 2, 2025

Hi Pavan,

I've looked at your PR, just wanted to check out the built documentation before commenting, which I haven't got a chance to do yet. I'll need some time to proofread the docs, will comment on this PR re my findings.

Sorry for the delay and thanks for doing this, much appreciated!

antond-weta
antond-weta reviewed Dec 2, 2025
View reviewed changes
Copy link
Contributor

@antond-weta antond-weta 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 overall, some minor errors.

pmady added 3 commits December 2, 2025 17:18
@pmady
Address review feedback on documentation
6c5b3b7 
- Remove Python bindings references (WIP, not fully functional)
- Fix rawtoaces_core.rst: remove non-existent classes (CameraDatabase,
  IlluminantDatabase, database_paths), add actual functions
- Elaborate on data path: mention CMAKE_INSTALL_PREFIX, Windows limitation
- Add --data-dir command line parameter to environment setup
- Fix illuminant docs: blackbody temperatures must be below 4000K

Signed-off-by: pmady <[email protected]>
@pmady
Address review feedback on documentation
0cbe173 
- Remove Python bindings references (WIP, not fully functional)
- Fix rawtoaces_core.rst: remove non-existent classes, add actual functions
- Elaborate on data path: mention CMAKE_INSTALL_PREFIX, Windows limitation
- Add --data-dir command line parameter to environment setup
- Fix illuminant docs: blackbody temperatures must be below 4000K
- Fix dependencies: remove LibRaw (handled by OIIO), add nlohmann-json
- Fix spectral_data.rst: explicitly list Spectrum and SpectralData structs

Signed-off-by: pmady <[email protected]>
@pmady
Fix API documentation: update image_converter and rawtoaces_core rst …
8fa95ac 
…files

Signed-off-by: pmady <[email protected]>
@pmady pmady requested a review from antond-weta December 2, 2025 23:34
@pmady
Copy link
Author

pmady commented Dec 2, 2025 •
edited
Loading

Updated PR with new changes

antond-weta
antond-weta reviewed Dec 3, 2025
View reviewed changes
docs/api/rawtoaces_core.rst
Copy link
Contributor

@antond-weta antond-weta Dec 3, 2025

Choose a reason for hiding this comment

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

MetadataSolver class is missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antond-weta antond-weta antond-weta left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pmady @codecov-commenter @antond-weta