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

feat: Added the rendercv_settings block #166

Merged
merged 31 commits into from
Sep 7, 2024
Merged

Conversation

akib1689
Copy link
Contributor

@akib1689 akib1689 commented Sep 1, 2024

As discussed in #146, A new block named rendercv_settings is added.

Usage of this block:

rendercv_settings:
  output_folder_name: 'str'
  use_local_latex_command: 'str'
  pdf_path: 'str'
  latex_path: 'str'
  html_path: 'str'
  png_path: 'str'
  markdown_path: 'str'
  no_html: false
  no_markdown: false
  no_png: false

Each and every property in this block is direct equivalent implementation of the cli option.

fixes: #146

akib1689 and others added 14 commits August 31, 2024 19:49
Now no error is thrown from the terminal when used the block `rendercv_settings`.
Though the functionality of the code block provided in the data model is not yet utilized.
This commit adds the `no_html` and `no_png` options to the `rendercv_settings` block in the `John_Doe_ClassicTheme_CV.yaml` file. These options allow the user to control whether the HTML and PNG files will be generated during the rendering process. The default values are set to `true` for `no_html` and `false` for `no_png`.
This commit refactors the code in `cli_command_render` to improve the handling of the output directory. The previous implementation used a hardcoded path, which has been replaced with a dynamic path based on the `output_folder_name` provided in the `rendercv_settings`. This change allows users to specify a custom output directory for the rendered files.
feat: Added render cv settings
rendercv/cli/commands.py Outdated Show resolved Hide resolved
rendercv/cli/printer.py Outdated Show resolved Hide resolved
rendercv/cli/commands.py Outdated Show resolved Hide resolved
@sinaatalay
Copy link
Owner

Thanks a lot for your great work! It looks really good. I've included some comments.

I'm quite tied up these days, so I might not be able to respond immediately, but I'll get back to you as soon as I can.

@sinaatalay sinaatalay marked this pull request as draft September 2, 2024 02:45
Copy link
Owner

@sinaatalay sinaatalay left a comment

Choose a reason for hiding this comment

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

When everything is all set, we need to write tests for every function created. Overriding the settings from YAML with CLI should be tested, too.

@sinaatalay sinaatalay assigned sinaatalay and unassigned sinaatalay Sep 2, 2024
@sinaatalay sinaatalay added the enhancement New feature or request label Sep 2, 2024
@akib1689
Copy link
Contributor Author

akib1689 commented Sep 3, 2024

I'll Use the comments as check-boxes and resolve them as I implement them. Is it okay? Once I resolve all the comments I'll use the ready for review.

@akib1689
Copy link
Contributor Author

akib1689 commented Sep 3, 2024

@sinaatalay Can you please review the changes now? I think most of the issue is resolved. and after your answer on the unresolved comments. I'll update and make the pr final.

@sinaatalay sinaatalay self-requested a review September 4, 2024 01:59
@sinaatalay sinaatalay marked this pull request as ready for review September 4, 2024 01:59
rendercv/cli/commands.py Outdated Show resolved Hide resolved
rendercv/cli/commands.py Outdated Show resolved Hide resolved
rendercv/cli/commands.py Outdated Show resolved Hide resolved
rendercv/cli/commands.py Outdated Show resolved Hide resolved
Copy link
Owner

@sinaatalay sinaatalay left a comment

Choose a reason for hiding this comment

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

Thank you! I have made a few more comments; then it will be ready. But we need tests. If you don't want to implement tests, I can do that too. If you want to work on it, let me know.

akib1689 and others added 2 commits September 4, 2024 09:45
…neration

This commit updates the `rendercv_settings` in the `John_Doe_ClassicTheme_CV.yaml` file to allow disabling the generation of HTML and PNG files. The `no_html` and `no_png` flags are set to `true` in the `render` section of the settings. This change provides more flexibility to users who may not need these file formats in their rendered output.
@akib1689
Copy link
Contributor Author

akib1689 commented Sep 4, 2024

Thank you! I have made a few more comments; then it will be ready. But we need tests. If you don't want to implement tests, I can do that too. If you want to work on it, let me know.

It would be very helpful. If you can implement testing as I'm not familiar with testing tools used in this project. But I'll surely can do documentation. All the change is implemented now. Only the documentation is remaining.

@sinaatalay
Copy link
Owner

Let me know when you have finished the pull request following our last discussion.

@akib1689 akib1689 marked this pull request as draft September 7, 2024 05:42
akib1689 and others added 3 commits September 7, 2024 06:37
This commit adds the `rendercv_settings` field to the YAML input file structure in the and the `rendercv_settings.md` describing the reference of the file.
@akib1689
Copy link
Contributor Author

akib1689 commented Sep 7, 2024

@sinaatalay Please review the documentation. and overall code structure. I think the pull request is ready for merging now.

@sinaatalay sinaatalay marked this pull request as ready for review September 7, 2024 06:41
Copy link
Owner

@sinaatalay sinaatalay left a comment

Choose a reason for hiding this comment

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

Thank you for your work! This is the most comprehensive contribution so far.

@sinaatalay sinaatalay merged commit 3a5d09b into sinaatalay:main Sep 7, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add rendercv_settings field to the data model
2 participants