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

docs: changed the notes on read_file. #896

Merged
merged 1 commit into from
Sep 28, 2023
Merged

docs: changed the notes on read_file. #896

merged 1 commit into from
Sep 28, 2023

Conversation

Jaskowicz1
Copy link
Contributor

@Jaskowicz1 Jaskowicz1 commented Sep 27, 2023

This PR alters the notes on dpp::utility::read_file. After experimenting in DPP-UE, I realised that there was no note as to what happens if the file is big and causes the event to timeout, causing a crash in DPP-UE and an error log in DPP. I made the note to give more information to users about this, making sure they know to use thinking if the file is big.

The PR also renames the previous note to be a warning. I feel that the warning is very much required so people know that the function can, and will, block the thread, if the file is big.

Documentation change checklist

  • My documentation changes follow the same style as the rest of the documentation and any examples follow the coding style guide.
  • I tested that my change works before raising the PR (via running doxygen, and testing examples).
  • I have not moved any existing pages or changed any existing URLs without strong justification as to why.
  • I have not generated content using AI or a desktop utility such as grammarly.

@netlify
Copy link

netlify bot commented Sep 27, 2023

Deploy Preview for dpp-dev ready!

Name Link
🔨 Latest commit b1b95fe
🔍 Latest deploy log https://app.netlify.com/sites/dpp-dev/deploys/65142263dc038a000889fa79
😎 Deploy Preview https://deploy-preview-896--dpp-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Sep 27, 2023
@Henonicks
Copy link
Contributor

You should also add the note to docs themselves. You are adding documentation anyways.

@Jaskowicz1
Copy link
Contributor Author

You should also add the note to docs themselves. You are adding documentation anyways.

? This is the docs themselves?

@Henonicks
Copy link
Contributor

? This is the docs themselves?

You've only edited the .h file but the dpp.dev docs will stay the same so the code documentation won't match.

@Jaskowicz1
Copy link
Contributor Author

? This is the docs themselves?

You've only edited the .h file but the dpp.dev docs will stay the same so the code documentation won't match.

Not true, the docs are auto generated from the .h.

@Henonicks
Copy link
Contributor

Mbad, didn't know that

@braindigitalis braindigitalis merged commit 0dbb22e into brainboxdotcc:dev Sep 28, 2023
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants