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

fix: fix image module docs #989

Closed
wants to merge 1 commit into from

Conversation

MadhurSaluja
Copy link

@MadhurSaluja MadhurSaluja commented Nov 21, 2024

Pull Request Description

Fixes Issue #954

This PR improves the Doxygen documentation for image.cpp and image.h. It adds detailed comments for functions, enums, and constants in the faker::image namespace, ensuring clear and comprehensive documentation.

Key Changes

  • Added Doxygen comments to all public functions in image.h.
  • Documented the ImageCategory enum with descriptions for each category.
  • Updated inline documentation in image.cpp for clarity.

How to Verify

  1. Run Doxygen to generate updated documentation.
  2. Verify the documentation for image.cpp and image.h in the generated docs/html files.

This resolves Issue #954.

Copy link
Owner

@cieslarmichal cieslarmichal left a comment

Choose a reason for hiding this comment

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

Well I am not sure about those changes, why did you changed so much in Doxyfile? (with removing app name and app logo), why did you add Doxyfile.bak to repo? and finally why do you document cpp files (we usually just put docs into headers)

Comment on lines -44 to +45
PROJECT_NAME = "Faker C++"
PROJECT_NAME = "My Project"
Copy link
Owner

Choose a reason for hiding this comment

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

why did you change it to my project?

@@ -60,19 +61,25 @@ PROJECT_BRIEF =
# pixels and the maximum width should not exceed 200 pixels. Doxygen will copy
# the logo to the output directory.

PROJECT_LOGO = docs/public/logo.png
Copy link
Owner

Choose a reason for hiding this comment

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

why did you remove logo?

Comment on lines +9 to +11
/**
* @brief Namespace for image-related functionalities.
*/
Copy link
Owner

Choose a reason for hiding this comment

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

no need to document namespace, please remove

Comment on lines +18 to +29
Animals, ///< Images related to animals.
Business, ///< Images related to business.
Cats, ///< Images of cats.
City, ///< Images of cityscapes.
Food, ///< Images of food items.
Nightlife, ///< Images of nightlife scenes.
Fashion, ///< Images related to fashion.
People, ///< Images of people.
Nature, ///< Images of nature.
Sports, ///< Images related to sports.
Technics, ///< Images of technical items.
Transport ///< Images of transport and vehicles.
Copy link
Owner

Choose a reason for hiding this comment

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

redundant comments

* @param width The width of the image. Default is 640.
* @param height The height of the image. Default is 480.
* @param category The optional category of the image.
* @return std::string The generated image URL.
Copy link
Owner

Choose a reason for hiding this comment

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

why did you remove code example?

@cieslarmichal
Copy link
Owner

closing because it is fixed in #998

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix image module docs
2 participants